🐛 Bug Report — `new Request(Proxy(request))` fails

[vicb]

• Ref issue on Open Next: [BUG] new Request(request, init) crashes in route handlers with Invalid URL: [object Request] opennextjs/opennextjs-cloudflare#1144
• Related Next.js code
• Tested with wrangler 4.86.0 / workerd 20260310

Creating a new Request from a proxy fails with TypeError: Invalid URL: [object Request]

Repro code on a worker:

export default {
	async fetch(): Promise<Response> {
		const handler = {
			get(_target, prop, _receiver) {
				console.log('get', prop);
				return Reflect.get(...arguments);
			},
		};

		const proxy = new Proxy(new Request('http://ex.com'), handler);
		const clone = new Request(proxy);
		console.log(clone.url);

		return new Response(`Clone URL: ${clone.url}`);
	},
} satisfies ExportedHandler<Env>;

The output is:

get Symbol(Symbol.toPrimitive)
get toString
get Symbol(Symbol.toStringTag)
[wrangler:error] TypeError: Invalid URL: [object Request]

The same code:

const handler = {
	get(_target, prop, _receiver) {
		console.log('get', prop);
		return Reflect.get(...arguments);
	},
};

const proxy = new Proxy(new Request('http://ex.com'), handler);
const clone = new Request(proxy);
console.log(clone.url);

Works well in Node and generate the following output:

get Symbol(dispatcher)
get Symbol(state)
get Symbol(signal)
get Symbol(state)
http://ex.com/

I believe this issue linked to #5476 and #5711 that implement something that does not match the Node.js behavior.

/cc @jasnell

[vicb]

The culprit might be around

workerd/src/workerd/jsg/iterator.h

Lines 557 to 592 in 5597141

	if (iter->IsNullOrUndefined()) {
	// Before checking for the sync iterator, let's also check to see if the object
	// implements a custom toString to Symbol.toPrimitive method that is not the default
	// Object.prototype.toString. If it does, then we won't treat it as
	// an iterator either. If the object is an Array, then we skip this check since
	// it's exceedingly uncommon for arrays to be subclassed with a custom toString method,
	// so much that it's not worth handling the extreme edge case.
	// This is to deal with edge cases around objects with customized stringify methods,
	// which are likely more common than those with customized iterator methods. While
	// these are both rare cases, it's better to err on the side of custom stringification
	// rather than custom iteration.
	if (config.fetchIterableTypeSupportOverrideAdjustment && !object->IsArray()) {
	if (protoToString == kj::none) {
	// TODO(cleanup): In several places in the codebase we have this pattern of
	// lazily grabbing the object prototype. We should probably centralize this
	// an cache it in the IsolateBase or something.
	auto obj = js.obj();
	auto proto = obj.getPrototype(js);
	protoToString = jsg::JsRef(
	js, KJ_ASSERT_NONNULL(proto.tryCast<jsg::JsObject>()).get(js, "toString"_kj));
	toPrimitiveString = jsg::JsRef(js,
	KJ_ASSERT_NONNULL(proto.tryCast<jsg::JsObject>()).get(js, js.symbolToPrimitive()));
	}
	// We only check that the toString/Symbol.toPrimitive is the same value as
	// Object.prototype.toString/Symbol.toPrimitive. This does not guarantee every
	// possible edge case but should be sufficient for our purposes.
	auto jsobj = JsObject(object);
	if (jsobj.get(js, "toString"_kj) != KJ_ASSERT_NONNULL(protoToString).getHandle(js) ||
	jsobj.get(js, js.symbolToPrimitive()) !=
	KJ_ASSERT_NONNULL(toPrimitiveString).getHandle(js)) {
	return kj::none;
	}
	}
	iter = check(object->Get(context, v8::Symbol::GetIterator(isolate)));

One important thing to note is that Next does return a bound function sometimes - then testing toString strict equality might not be the best strategy here.

Anyway happy to help testing a candidate PR

[kentonv]

I don't think this has anything to do with iterators or the fetchIterableTypeSupport flag. The reason you're seeing it probe for stringification is just because it's being stringified in the error mesasge.

Rather, the problem here is that fetch()'s first parameter is defined to be Request | string. We expect either a native Request object, or a string URL.

A Proxy is not a native Request object. It's a Proxy.

Since fetch is a native function, it cannot work with a Proxy.

This doesn't work in Google Chrome either. The reason it works in Node is because fetch() is not a native function in Node -- it's implemented in JavaScript. Because of this, a Proxy is able to pretend to be a Request object. But this would be difficult to support in fetch() implemented as native code because the native code does not call back into JavaScript when accessing properties, so cannot honor the Proxy's handlers.

To make this work with a native function would be difficult.

IMO this is a bug in Next. Passing proxies to native APIs is not meant to work -- it coincidentally works in Node only because fetch() isn't native there.

[vicb]

@kentonv I'm not sure to understand what fetch has to do with this?

[jasnell]

@vicb:

I'm not sure to understand what fetch has to do with this?

Request is part of the fetch spec.

@kentonv:

... This doesn't work in Google Chrome either.

Hmm.. seems to work for me in Edge, which uses the same implementation as chrome

...To make this work with a native function would be difficult.

It shouldn't be too difficult really. Bit convoluted since we'd have to detect that it's a v8::Proxy and run all operations through the traps, but overall not too hard. Performance would be lousy, but it's doable.

[kentonv]

Hmm.. seems to work for me in Edge, which uses the same implementation as chrome

Look more closely. It didn't work. The proxy you passed into new Request was not interpreted as being a Request object, so instead the implementation decided it must be a URL and coerced it to a string. That's why clone.url came back as https://<page-you-were-on>/[object%20Request] instead of http://ex.com.

Bit convoluted since we'd have to detect that it's a v8::Proxy and run all operations through the traps, but overall not too hard.

We'd need an entire separate code path for this that reads the properties of the input object as JS properties. Seems pretty convoluted to me.

Though maybe a simple approach could be to run it through new Request(proxy.url, proxy), which should inherently recreate the request based on all the properties of proxy.

[kentonv]

I'm not sure to understand what fetch has to do with this?

The parameters to new Request (the constructor) are handled the same as the parameters to fetch.

[jasnell]

There's another issue with handling jsg::Object instances via Proxy that's unrelated to this specific issue but there's a fairly simple way to handle it. #6730 ... not that convoluted.