From b7e616e6ca516af38bb89ead0274a12b564e7e6b Mon Sep 17 00:00:00 2001 From: Harshal Brahmbhatt Date: Tue, 27 Aug 2024 14:26:16 +0000 Subject: [PATCH] Fix R2 wildcard etag parsing This commit addresses issue #2572 --- src/workerd/api/r2-bucket.c++ | 19 +++++-- src/workerd/api/r2-test.js | 92 +++++++++++++++++++++++++++++++++ src/workerd/api/r2-test.wd-test | 19 +++++++ 3 files changed, 125 insertions(+), 5 deletions(-) create mode 100644 src/workerd/api/r2-test.js create mode 100644 src/workerd/api/r2-test.wd-test diff --git a/src/workerd/api/r2-bucket.c++ b/src/workerd/api/r2-bucket.c++ index 1428f5919b7f..93670ff96d44 100644 --- a/src/workerd/api/r2-bucket.c++ +++ b/src/workerd/api/r2-bucket.c++ @@ -935,10 +935,19 @@ kj::Array parseConditionalEtagHeader(kj::StringPtr condHeader, } } -kj::Array buildSingleStrongEtagArray(kj::StringPtr etagValue) { - struct R2Bucket::StrongEtag etag = {.value = kj::str(etagValue)}; +kj::Array buildSingleEtagArray(kj::StringPtr etagValue) { + auto value = kj::str(etagValue); + kj::ArrayBuilder etagArrayBuilder = kj::heapArrayBuilder(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(); } @@ -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; diff --git a/src/workerd/api/r2-test.js b/src/workerd/api/r2-test.js new file mode 100644 index 000000000000..657ac73b8120 --- /dev/null +++ b/src/workerd/api/r2-test.js @@ -0,0 +1,92 @@ +// 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: "*", + }, + }); + }, +}; diff --git a/src/workerd/api/r2-test.wd-test b/src/workerd/api/r2-test.wd-test new file mode 100644 index 000000000000..52703f7be0aa --- /dev/null +++ b/src/workerd/api/r2-test.wd-test @@ -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"], + ) + ), + ], +);