Skip to content

Commit

Permalink
Improve error message for R2 etag parsing logic (#2233)
Browse files Browse the repository at this point in the history
  • Loading branch information
Frederik-Baetens authored Jun 7, 2024
1 parent 5a29a78 commit f2878ac
Showing 1 changed file with 21 additions and 2 deletions.
23 changes: 21 additions & 2 deletions src/workerd/api/r2-bucket.c++
Original file line number Diff line number Diff line change
Expand Up @@ -846,8 +846,27 @@ kj::Array<R2Bucket::Etag> parseConditionalEtagHeader(
// which just results in an empty list if it's out of bounds by 1.
return parseConditionalEtagHeader(condHeader.slice(nextComma + 1), kj::mv(etagAccumulator));
} else if (leadingCommaRequired) {
// Did not find a leading comma, and we expected a leading comma before any further etags
JSG_FAIL_REQUIRE(Error, "Comma was expected to separate etags");
// we don't need to include nextComma in this min check since in this else branch nextComma is
// always larger than at least one of nextWildcard, nextQuotation and nextWeak
size_t firstEncounteredProblem = std::min({nextWildcard, nextQuotation, nextWeak});

kj::String failureReason;
if (firstEncounteredProblem == nextWildcard) {
failureReason = kj::str("Encountered a wildcard character '*' instead.");
} else if (firstEncounteredProblem == nextQuotation) {
failureReason = kj::str(
"Encountered a double quote character '\"' instead. "
"This would otherwise indicate the start of a new strong etag.");
} else if (firstEncounteredProblem == nextWeak) {
failureReason = kj::str(
"Encountered a weak quotation character 'W' instead. "
"This would otherwise indicate the start of a new weak etag.");
} else {
KJ_FAIL_ASSERT("We shouldn't be able to reach this point. The above etag parsing code is incorrect.");
}

// Did not find a leading comma, and we expected a leading comma before any further etags
JSG_FAIL_REQUIRE(Error, "Comma was expected to separate etags. ", failureReason);
}

if (nextWildcard < nextQuotation) {
Expand Down

0 comments on commit f2878ac

Please sign in to comment.