Skip to content

Commit

Permalink
Fix R2 wildcard etag parsing
Browse files Browse the repository at this point in the history
This commit addresses issue cloudflare#2572
  • Loading branch information
harshal317 committed Aug 27, 2024
1 parent 58c83c1 commit f683c18
Show file tree
Hide file tree
Showing 3 changed files with 122 additions and 5 deletions.
19 changes: 14 additions & 5 deletions src/workerd/api/r2-bucket.c++
Original file line number Diff line number Diff line change
Expand Up @@ -935,10 +935,19 @@ kj::Array<R2Bucket::Etag> parseConditionalEtagHeader(kj::StringPtr condHeader,
}
}

kj::Array<R2Bucket::Etag> buildSingleStrongEtagArray(kj::StringPtr etagValue) {
struct R2Bucket::StrongEtag etag = {.value = kj::str(etagValue)};
kj::Array<R2Bucket::Etag> buildSingleEtagArray(kj::StringPtr etagValue) {
auto value = kj::str(etagValue);

kj::ArrayBuilder<R2Bucket::Etag> etagArrayBuilder = kj::heapArrayBuilder<R2Bucket::Etag>(1);
etagArrayBuilder.add(kj::mv(etag));

if (value == "*") {
struct R2Bucket::WildcardEtag etag = {};
etagArrayBuilder.add(kj::mv(etag));
} else {
struct R2Bucket::StrongEtag etag = {.value = kj::str(etagValue)};
etagArrayBuilder.add(kj::mv(etag));
}

return etagArrayBuilder.finish();
}

Expand Down Expand Up @@ -967,12 +976,12 @@ R2Bucket::UnwrappedConditional::UnwrappedConditional(const Conditional& c)
KJ_IF_SOME(e, c.etagMatches) {
JSG_REQUIRE(!isQuotedEtag(e.value), TypeError,
"Conditional ETag should not be wrapped in quotes (", e.value, ").");
etagMatches = buildSingleStrongEtagArray(e.value);
etagMatches = buildSingleEtagArray(e.value);
}
KJ_IF_SOME(e, c.etagDoesNotMatch) {
JSG_REQUIRE(!isQuotedEtag(e.value), TypeError,
"Conditional ETag should not be wrapped in quotes (", e.value, ").");
etagDoesNotMatch = buildSingleStrongEtagArray(e.value);
etagDoesNotMatch = buildSingleEtagArray(e.value);
}
KJ_IF_SOME(d, c.uploadedAfter) {
uploadedAfter = d;
Expand Down
89 changes: 89 additions & 0 deletions src/workerd/api/r2-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
// Copyright (c) 2023 Cloudflare, Inc.
// Licensed under the Apache 2.0 license found in the LICENSE file or at:
// https://opensource.org/licenses/Apache-2.0

import assert from 'node:assert';

export default {
// Handler for HTTP request binding makes to R2
async fetch(request, env, ctx) {
// We only expect PUT/Get
assert(request.method === "PUT" || request.method === "GET")

// Each request should have a metadata size header indicating how much
// we should read to understand what type of request this is
const metadataSizeString = request.headers.get("cf-r2-metadata-size")
assert(metadataSizeString !== null)
const metadataSize = parseInt(metadataSizeString)
assert(!Number.isNaN(metadataSize))

const reader = request.body.getReader({ mode: 'byob' })
const jsonArray = new Uint8Array(metadataSize)
const { value } = await reader.readAtLeast(metadataSize, jsonArray)
const jsonRequest = JSON.parse(new TextDecoder().decode(value))

// Currently not using the body in these test so I'm going to just discard
while (true) {
const read = await reader.read(new Uint8Array(65536))
if (!read.done) {
continue
} else {
break
}
}

// Assert it's the correct version
assert(jsonRequest.version = 1)

if (request.method === "PUT") {
assert(jsonRequest.method === "put")

if (jsonRequest.object === "onlyIfStrongEtag") {
assert(jsonRequest.onlyIf.etagMatches.length === 1)
assert(jsonRequest.onlyIf.etagDoesNotMatch.length === 1)

assert(jsonRequest.onlyIf.etagMatches[0].type === "strong")
assert(jsonRequest.onlyIf.etagDoesNotMatch[0].type === "strong")

assert(jsonRequest.onlyIf.etagMatches[0].value === "strongEtag")
assert(jsonRequest.onlyIf.etagDoesNotMatch[0].value === "strongEtag")
}

if (jsonRequest.object === "onlyIfWildcard") {
assert(jsonRequest.onlyIf.etagMatches.length === 1)
assert(jsonRequest.onlyIf.etagDoesNotMatch.length === 1)

assert(jsonRequest.onlyIf.etagMatches[0].type === "wildcard")
assert(jsonRequest.onlyIf.etagDoesNotMatch[0].type === "wildcard")
}

return new Response(new TextEncoder().encode(JSON.stringify({
name: "objectKey",
version : "objectVersion",
size: "123",
etag: "objectEtag",
uploaded: "1724767257918",
storageClass: "Standard"
})))
}

throw new Error("unexpected")
},


async test(ctrl, env, ctx) {
await env.BUCKET.put("basic", "content");
await env.BUCKET.put("onlyIfStrongEtag", "content", {
onlyIf: {
etagMatches: "strongEtag",
etagDoesNotMatch: "strongEtag"
}
})
await env.BUCKET.put("onlyIfWildcard", "content", {
onlyIf: {
etagMatches: "*",
etagDoesNotMatch: "*"
}
})
},
};
19 changes: 19 additions & 0 deletions src/workerd/api/r2-test.wd-test
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
using Workerd = import "/workerd/workerd.capnp";

const unitTests :Workerd.Config = (
services = [
( name = "r2-test",
worker = (
modules = [
( name = "worker", esModule = embed "r2-test.js" )
],
bindings = [
( name = "BUCKET", r2Bucket = "r2-test" ),
( name = "SERVICE", service = "r2-test" ),
],
compatibilityDate = "2023-07-24",
compatibilityFlags = ["nodejs_compat", "service_binding_extra_handlers"],
)
),
],
);

0 comments on commit f683c18

Please sign in to comment.