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 Sep 6, 2024
1 parent 4faf04b commit d5b13a4
Show file tree
Hide file tree
Showing 3 changed files with 129 additions and 5 deletions.
17 changes: 12 additions & 5 deletions src/workerd/api/r2-bucket.c++
Original file line number Diff line number Diff line change
Expand Up @@ -935,10 +935,17 @@ 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) {
kj::ArrayBuilder<R2Bucket::Etag> etagArrayBuilder = kj::heapArrayBuilder<R2Bucket::Etag>(1);
etagArrayBuilder.add(kj::mv(etag));

if (etagValue == "*") {
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 +974,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
98 changes: 98 additions & 0 deletions src/workerd/api/r2-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
// 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(['GET', 'PUT'].includes(request.method));

// 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.notStrictEqual(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);
reader.releaseLock();

const jsonRequest = JSON.parse(new TextDecoder().decode(value));

// Currently not using the body in these test so I'm going to just discard
for await (const _ of request.body) {
}

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

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

if (jsonRequest.object === 'onlyIfStrongEtag') {
assert.deepStrictEqual(jsonRequest.onlyIf, {
etagMatches: [
{
value: 'strongEtag',
type: 'strong',
},
],
etagDoesNotMatch: [
{
value: 'strongEtag',
type: 'strong',
},
],
});
}

if (jsonRequest.object === 'onlyIfWildcard') {
assert.deepStrictEqual(jsonRequest.onlyIf, {
etagMatches: [
{
type: 'wildcard',
},
],
etagDoesNotMatch: [
{
type: 'wildcard',
},
],
});
}

return Response.json({
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 d5b13a4

Please sign in to comment.