Skip to content

Commit

Permalink
Remove admin account tag from path in R2 operations
Browse files Browse the repository at this point in the history 
The admin account path is not needed. This was an oversight from cloudflare#577.
  • Loading branch information
@OilyLime
OilyLime committed May 25, 2023
1 parent 0d2b68c commit 9b910c5
Show file tree
Hide file tree
Showing 6 changed files with 35 additions and 46 deletions.
17 changes: 5 additions & 12 deletions src/workerd/api/r2-admin.c++
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,9 @@

namespace workerd::api::public_beta {
jsg::Ref<R2Bucket> R2Admin::get(jsg::Lock& js, kj::String bucketName) {
KJ_IF_MAYBE(a, adminAccount) {
auto& j = KJ_ASSERT_NONNULL(jwt, "adminAccount without corresponding jwt");
KJ_IF_MAYBE(j, jwt) {
return jsg::alloc<R2Bucket>(featureFlags, subrequestChannel, kj::mv(bucketName),
kj::str(*a), kj::str(j), R2Bucket::friend_tag_t{});
kj::str(j), R2Bucket::friend_tag_t{});
}
return jsg::alloc<R2Bucket>(featureFlags, subrequestChannel, kj::mv(bucketName), R2Bucket::friend_tag_t{});
}
Expand All @@ -36,10 +35,8 @@ jsg::Promise<jsg::Ref<R2Bucket>> R2Admin::create(jsg::Lock& js, kj::String name,
createBucketBuilder.setBucket(name);

auto requestJson = json.encode(requestBuilder);
kj::StringPtr components[2];
auto path = fillR2Path(components, adminAccount, nullptr);
auto promise = doR2HTTPPutRequest(js, kj::mv(client), nullptr, nullptr,
kj::mv(requestJson), path, jwt);
kj::mv(requestJson), nullptr, jwt);

return context.awaitIo(kj::mv(promise),
[this, subrequestChannel = subrequestChannel, name = kj::mv(name), &errorType]
Expand Down Expand Up @@ -76,9 +73,7 @@ jsg::Promise<R2Admin::ListResult> R2Admin::list(jsg::Lock& js,
}

auto requestJson = json.encode(requestBuilder);
kj::StringPtr components[2];
auto path = fillR2Path(components, adminAccount, nullptr);
auto promise = doR2HTTPGetRequest(kj::mv(client), kj::mv(requestJson), path, jwt);
auto promise = doR2HTTPGetRequest(kj::mv(client), kj::mv(requestJson), nullptr, jwt);

return context.awaitIo(js, kj::mv(promise),
[this, &retrievedBucketType, &errorType](jsg::Lock& js, R2Result r2Result) mutable {
Expand Down Expand Up @@ -131,10 +126,8 @@ jsg::Promise<void> R2Admin::delete_(jsg::Lock& js, kj::String name,
deleteBucketBuilder.setBucket(name);

auto requestJson = json.encode(requestBuilder);
kj::StringPtr components[2];
auto path = fillR2Path(components, adminAccount, nullptr);
auto promise = doR2HTTPPutRequest(js, kj::mv(client), nullptr, nullptr,
kj::mv(requestJson), path, jwt);
kj::mv(requestJson), nullptr, jwt);

return context.awaitIo(kj::mv(promise), [&errorType](R2Result r2Result) mutable {
r2Result.throwIfError("deleteBucket", errorType);
Expand Down
3 changes: 0 additions & 3 deletions src/workerd/api/r2-admin.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,10 @@ class R2Admin: public jsg::Object {

R2Admin(FeatureFlags featureFlags,
uint subrequestChannel,
kj::String account,
kj::String jwt,
friend_tag_t)
: featureFlags(featureFlags),
subrequestChannel(subrequestChannel),
adminAccount(kj::mv(account)),
jwt(kj::mv(jwt)) {}
// This constructor is intended to be used by the R2CrossAccount binding, which has access to the
// friend_tag
Expand Down Expand Up @@ -100,7 +98,6 @@ class R2Admin: public jsg::Object {
private:
R2Bucket::FeatureFlags featureFlags;
uint subrequestChannel;
kj::Maybe<kj::String> adminAccount;
kj::Maybe<kj::String> jwt;

friend class edgeworker::api::R2CrossAccount;
Expand Down
29 changes: 13 additions & 16 deletions src/workerd/api/r2-bucket.c++
View file
Original file line number Diff line number Diff line change
Expand Up @@ -328,8 +328,8 @@ jsg::Promise<kj::Maybe<jsg::Ref<R2Bucket::HeadResult>>> R2Bucket::head(
headBuilder.setObject(name);

auto requestJson = json.encode(requestBuilder);
kj::StringPtr components[2];
auto path = fillR2Path(components, adminAccount, adminBucket);
kj::StringPtr components[1];
auto path = fillR2Path(components, adminBucket);
auto promise = doR2HTTPGetRequest(kj::mv(client), kj::mv(requestJson), path, jwt);

return context.awaitIo(kj::mv(promise), [&errorType](R2Result r2Result) {
Expand Down Expand Up @@ -364,8 +364,8 @@ R2Bucket::get(jsg::Lock& js, kj::String name, jsg::Optional<GetOptions> options,
initGetOptions(js, getBuilder, *o);
}
auto requestJson = json.encode(requestBuilder);
kj::StringPtr components[2];
auto path = fillR2Path(components, adminAccount, adminBucket);
kj::StringPtr components[1];
auto path = fillR2Path(components, adminBucket);
auto promise = doR2HTTPGetRequest(kj::mv(client), kj::mv(requestJson), path, jwt);

return context.awaitIo(kj::mv(promise), [&context, &errorType](R2Result r2Result)
Expand Down Expand Up @@ -554,8 +554,8 @@ R2Bucket::put(jsg::Lock& js, kj::String name, kj::Maybe<R2PutValue> value,
auto requestJson = json.encode(requestBuilder);

cancelReader.cancel();
kj::StringPtr components[2];
auto path = fillR2Path(components, adminAccount, adminBucket);
kj::StringPtr components[1];
auto path = fillR2Path(components, adminBucket);
auto promise = doR2HTTPPutRequest(js, kj::mv(client), kj::mv(value), nullptr,
kj::mv(requestJson), path, jwt);

Expand Down Expand Up @@ -640,8 +640,8 @@ jsg::Promise<jsg::Ref<R2MultipartUpload>> R2Bucket::createMultipartUpload(jsg::L
}

auto requestJson = json.encode(requestBuilder);
kj::StringPtr components[2];
auto path = fillR2Path(components, adminAccount, adminBucket);
kj::StringPtr components[1];
auto path = fillR2Path(components, adminBucket);
auto promise = doR2HTTPPutRequest(js, kj::mv(client), nullptr, nullptr, kj::mv(requestJson),
path, jwt);

Expand Down Expand Up @@ -694,8 +694,8 @@ jsg::Promise<void> R2Bucket::delete_(jsg::Lock& js, kj::OneOf<kj::String, kj::Ar

auto requestJson = json.encode(requestBuilder);

kj::StringPtr components[2];
auto path = fillR2Path(components, adminAccount, adminBucket);
kj::StringPtr components[1];
auto path = fillR2Path(components, adminBucket);
auto promise = doR2HTTPPutRequest(js, kj::mv(client), nullptr, nullptr, kj::mv(requestJson),
path, jwt);

Expand Down Expand Up @@ -795,8 +795,8 @@ jsg::Promise<R2Bucket::ListResult> R2Bucket::list(

auto requestJson = json.encode(requestBuilder);

kj::StringPtr components[2];
auto path = fillR2Path(components, adminAccount, adminBucket);
kj::StringPtr components[1];
auto path = fillR2Path(components, adminBucket);
auto promise = doR2HTTPGetRequest(kj::mv(client), kj::mv(requestJson), path, jwt);

return context.awaitIo(kj::mv(promise),
Expand Down Expand Up @@ -1100,12 +1100,9 @@ kj::Maybe<jsg::Ref<R2Bucket::HeadResult>> parseHeadResultWrapper(
return parseObjectMetadata<R2Bucket::HeadResult>(action, r2Result, errorType);
}

kj::ArrayPtr<kj::StringPtr> fillR2Path(kj::StringPtr pathStorage[2], const kj::Maybe<kj::String>& account, const kj::Maybe<kj::String>& bucket) {
kj::ArrayPtr<kj::StringPtr> fillR2Path(kj::StringPtr pathStorage[1], const kj::Maybe<kj::String>& bucket) {
int numComponents = 0;

KJ_IF_MAYBE(a, account) {
pathStorage[numComponents++] = *a;
}
KJ_IF_MAYBE(b, bucket) {
pathStorage[numComponents++] = *b;
}
Expand Down
7 changes: 3 additions & 4 deletions src/workerd/api/r2-bucket.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
namespace workerd::api::public_beta {

kj::Array<kj::byte> cloneByteArray(const kj::Array<kj::byte>& arr);
kj::ArrayPtr<kj::StringPtr> fillR2Path(kj::StringPtr pathStorage[2], const kj::Maybe<kj::String>& admin, const kj::Maybe<kj::String>& bucket);
kj::ArrayPtr<kj::StringPtr> fillR2Path(kj::StringPtr pathStorage[1], const kj::Maybe<kj::String>& bucket);

class R2MultipartUpload;

Expand All @@ -40,8 +40,8 @@ class R2Bucket: public jsg::Object {
explicit R2Bucket(FeatureFlags featureFlags, uint clientIndex, kj::String bucket, friend_tag_t)
: featureFlags(featureFlags), clientIndex(clientIndex), adminBucket(kj::mv(bucket)) {}

explicit R2Bucket(FeatureFlags featureFlags, uint clientIndex, kj::String bucket, kj::String account, kj::String jwt, friend_tag_t)
: featureFlags(featureFlags), clientIndex(clientIndex), adminBucket(kj::mv(bucket)), adminAccount(kj::mv(account)), jwt(kj::mv(jwt)) {}
explicit R2Bucket(FeatureFlags featureFlags, uint clientIndex, kj::String bucket, kj::String jwt, friend_tag_t)
: featureFlags(featureFlags), clientIndex(clientIndex), adminBucket(kj::mv(bucket)), jwt(kj::mv(jwt)) {}

struct Range {
jsg::Optional<double> offset;
Expand Down Expand Up @@ -407,7 +407,6 @@ class R2Bucket: public jsg::Object {
FeatureFlags featureFlags;
uint clientIndex;
kj::Maybe<kj::String> adminBucket;
kj::Maybe<kj::String> adminAccount;
kj::Maybe<kj::String> jwt;

friend class R2Admin;
Expand Down
13 changes: 6 additions & 7 deletions src/workerd/api/r2-multipart.c++
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,9 @@ jsg::Promise<R2MultipartUpload::UploadedPart> R2MultipartUpload::uploadPart(
uploadPartBuilder.setObject(key);

auto requestJson = json.encode(requestBuilder);
auto bucket = this->bucket->adminBucket.map([](auto&& s) { return kj::str(s); });

kj::StringPtr components[2];
auto path = fillR2Path(components, nullptr, this->bucket->adminBucket);
kj::StringPtr components[1];
auto path = fillR2Path(components, this->bucket->adminBucket);
auto promise = doR2HTTPPutRequest(js, kj::mv(client), kj::mv(value), nullptr,
kj::mv(requestJson), path, nullptr);

Expand Down Expand Up @@ -99,8 +98,8 @@ jsg::Promise<jsg::Ref<R2Bucket::HeadResult>> R2MultipartUpload::complete(

auto requestJson = json.encode(requestBuilder);

kj::StringPtr components[2];
auto path = fillR2Path(components, nullptr, this->bucket->adminBucket);
kj::StringPtr components[1];
auto path = fillR2Path(components, this->bucket->adminBucket);
auto promise = doR2HTTPPutRequest(js, kj::mv(client), nullptr, nullptr, kj::mv(requestJson),
path, nullptr);

Expand Down Expand Up @@ -135,8 +134,8 @@ jsg::Promise<void> R2MultipartUpload::abort(jsg::Lock& js, const jsg::TypeHandle

auto requestJson = json.encode(requestBuilder);

kj::StringPtr components[2];
auto path = fillR2Path(components, nullptr, this->bucket->adminBucket);
kj::StringPtr components[1];
auto path = fillR2Path(components, this->bucket->adminBucket);
auto promise = doR2HTTPPutRequest(js, kj::mv(client), nullptr, nullptr, kj::mv(requestJson),
path, nullptr);

Expand Down
12 changes: 8 additions & 4 deletions src/workerd/api/r2-rpc.c++
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,10 @@ kj::Promise<R2Result> doR2HTTPGetRequest(kj::Own<kj::HttpClient> client,
kj::Url url;
url.scheme = kj::str("https");
url.host = kj::str("fake-host");
for (const auto &p : path) {
url.path.add(kj::str(p));
if (path != nullptr) {
for (const auto &p : path) {
url.path.add(kj::str(p));
}
}

auto& headerIds = context.getHeaderIds();
Expand Down Expand Up @@ -141,8 +143,10 @@ kj::Promise<R2Result> doR2HTTPPutRequest(jsg::Lock& js, kj::Own<kj::HttpClient>
kj::Url url;
url.scheme = kj::str("https");
url.host = kj::str("fake-host");
for (const auto &p : path) {
url.path.add(kj::str(p));
if (path != nullptr) {
for (const auto &p : path) {
url.path.add(kj::str(p));
}
}

kj::Maybe<uint64_t> expectedBodySize;
Expand Down

0 comments on commit 9b910c5

Please sign in to comment.