Skip to content

Commit

Permalink
Address nits
Browse files Browse the repository at this point in the history
  • Loading branch information
AdityaAtulTewari committed Aug 1, 2024
1 parent 8ee2cc9 commit 5aa3639
Show file tree
Hide file tree
Showing 6 changed files with 34 additions and 25 deletions.
14 changes: 6 additions & 8 deletions src/workerd/api/http-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -270,14 +270,12 @@ export const cacheMode = {
assertFetchCacheRejectsError('no-transform');
assertFetchCacheRejectsError('unsupported');
} else {
{
const req = new Request('https://example.org', { cache: 'no-store' });
assert.strictEqual(req.cache, 'no-store');
}
{
const req = new Request('https://example.org', { cache: 'no-cache' });
assert.strictEqual(req.cache, 'no-cache');
}
assertRequestCacheThrowsError('no-store',
'TypeError',
"Unsupported cache mode: no-store");
assertRequestCacheThrowsError('no-cache',
'TypeError',
"Unsupported cache mode: no-cache");
assertRequestCacheThrowsError('no-transform',
'TypeError',
"Unsupported cache mode: no-transform");
Expand Down
17 changes: 5 additions & 12 deletions src/workerd/api/http.c++
Original file line number Diff line number Diff line change
Expand Up @@ -1198,14 +1198,13 @@ kj::Maybe<kj::String> Request::serializeCfBlobJson(jsg::Lock& js) {
return cf.serialize(js);
}

void RequestInitializerDict::validate() {
void RequestInitializerDict::validate(jsg::Lock& js) {
KJ_IF_SOME(c, cache){
// Check compatability
JSG_REQUIRE(Worker::Api::current().getFeatureFlags().getCacheOptionEnabled(), Error, kj::str(
// Check compatability flag
JSG_REQUIRE(FeatureFlags::get(js).getCacheOptionEnabled(), Error, kj::str(
"The 'cache' field on 'RequestInitializerDict' is not implemented."));

// Check that the string is a supported type
auto cacheStruct = getCacheModeFromName(c);
JSG_FAIL_REQUIRE(TypeError, kj::str("Unsupported cache mode: ", c));
}
}

Expand Down Expand Up @@ -1818,13 +1817,7 @@ jsg::Promise<jsg::Ref<Response>> fetchImplNoOutputLock(
// If the jsRequest has a CacheMode, we need to handle that here.
// Currently, the only cache mode we support is undefined, but we will soon support
// no-cache and no-store. These additional modes will be hidden behind an autogate.
if (jsRequest->getCacheMode() != Request::CacheMode::NONE) {
JSG_REQUIRE(FeatureFlags::get(js).getCacheOptionEnabled(), Error, kj::str(
+ "The 'cache' field on 'RequestInitializerDict' is not implemented."));
return js.rejectedPromise<jsg::Ref<Response>>(
js.typeError(kj::str("Unsupported cache mode: ",
KJ_ASSERT_NONNULL(jsRequest->getCache(js)))));
}
KJ_ASSERT(jsRequest->getCacheMode() == Request::CacheMode::NONE);

kj::String url = uriEncodeControlChars(
urlList.back().toString(kj::Url::HTTP_PROXY_REQUEST).asBytes());
Expand Down
6 changes: 4 additions & 2 deletions src/workerd/api/http.h
Original file line number Diff line number Diff line change
Expand Up @@ -752,15 +752,17 @@ struct RequestInitializerDict {
// jsg::Optional<kj::String> priority;
// TODO(conform): Might support later?

void validate();

JSG_STRUCT(method, headers, body, redirect, fetcher, cf, mode, credentials, cache,
referrer, referrerPolicy, integrity, signal);
JSG_STRUCT_TS_OVERRIDE(RequestInit<Cf = CfProperties> {
headers?: HeadersInit;
body?: BodyInit | null;
cf?: Cf;
});

// This method is called within tryUnwrap() when the type is unpacked from v8.
// See jsg Readme for more details.
void validate(jsg::Lock&);
};

class Request: public Body {
Expand Down
3 changes: 2 additions & 1 deletion src/workerd/jsg/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -642,12 +642,13 @@ included in the JavaScript object.)

If the struct has a validate() method, it is called when the struct is unwrapped from v8.
This is an opportunity for it to throw a TypeError based on some custom logic.
The signature for this method is `void validate(jsg::Lock&);`

```cpp
struct ValdiatingFoo {
kj::String abc;

void validate() {
void validate(jsg::Lock& lock) {
JSG_REQUIRE(abc.size() != 0, TypeError, "Field 'abc' had no length in 'ValidatingFoo'.");
}

Expand Down
11 changes: 11 additions & 0 deletions src/workerd/jsg/jsg.h
Original file line number Diff line number Diff line change
Expand Up @@ -639,6 +639,17 @@ namespace {
//
// Note that if a `validate` function is provided, then it will be called after the struct is
// unwrapped from v8. This would be an appropriate time to throw an error.
// Signature: void validate(jsg::Lock& js);
// Example:
// struct ValdiatingFoo {
// kj::String abc;
// void validate(jsg::Lock& js) {
// JSG_REQUIRE(abc.size() != 0, TypeError, "Field 'abc' had no length in 'ValidatingFoo'.");
// }
// JSG_STRUCT(abc);
// };
//
// In this example the validate method would throw a `TypeError` if the size of the `abc` field was zero.
//
// Fields with a starting '$' will have that dollar sign prefix stripped in the JS binding. A
// motivating example to enact that change was WebCrypto which has a field in a dictionary called
Expand Down
8 changes: 6 additions & 2 deletions src/workerd/jsg/struct.h
Original file line number Diff line number Diff line change
Expand Up @@ -122,8 +122,12 @@ class StructWrapper<Self, T, TypeTuple<FieldWrappers...>, kj::_::Indexes<indices
kj::get<indices>(fields).unwrap(static_cast<Self&>(*this), isolate, context, in)...
};

if constexpr (requires { t.validate(); }) {
t.validate();
// Note that if a `validate` function is provided, then it will be called after the struct is
// unwrapped from v8. This would be an appropriate time to throw an error.
// Signature: void validate(jsg::Lock& js);
if constexpr (requires (jsg::Lock& js) { t.validate(js); }) {
jsg::Lock& js = jsg::Lock::from(isolate);
t.validate(js);
}

return t;
Expand Down

0 comments on commit 5aa3639

Please sign in to comment.