New issue
Advanced search Search tips

Issue 868888 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner: ----
Closed: Jul 30
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 2
Type: Bug



Sign in to add a comment

Implicit null -> string conversion for RequestInfo APIs

Project Member Reported by peter@chromium.org, Jul 30

Issue description

Consider the following two examples:

    await fetch(null);
    await swRegistration.backgroundFetch.fetch('my-id', null);

In both cases, they will attempt to fetch a file named "null" relative to the current path. I wouldn't expect that to happen.

Internally, both APIs use a union type ("RequestInfo") that's either a Request instance or a USVString. Our binding conversion code[1][2] doesn't recognize this as a nullable type, so falls back to forcefully converting it to a string. Hence this result.

What's the correct path forward here? I'd like "NULL" to be treated differently for Background Fetch, but this doesn't feel like an intuitive result for either API.


[1] https://cs.chromium.org/chromium/src/out/Debug/gen/third_party/blink/renderer/bindings/core/v8/request_or_usv_string.cc?type=cs&g=0&l=65
[2] https://cs.chromium.org/chromium/src/out/Debug/gen/third_party/blink/renderer/bindings/modules/v8/request_or_usv_string_or_request_or_usv_string_sequence.cc?type=cs&sq=package:chromium&g=0&l=84
 
Same with `undefined` - fetch(undefined) fetches a file called "undefined".

Firefox rejects the request with a TypeError because null/undefined is not a valid URL.
I think this would require a fetch spec change.  The nullability is dictated by the webidl defined in the spec.  The constructor for Request also throws if it gets a malformed URL, so that check would have to be bypassed somehow as well.
Expanding on wanderview's comment (I ended up writing this before his post):

AFAICS the conversion seems to be following the WebIDL spec correctly. RequestInfo is defined as

    (Request or USVString)

and fetch()'s prototype is

    fetch(RequestInfo input, optional RequestInit init)

|input| is not nullable (i.e. its type is RequestInfo rather than RequestInfo?), and the union itself does not contain any nullable types. Per https://heycam.github.io/webidl/#es-union all other tests fail and we end up in step 14 ("If types includes a string type, then return the result of converting V to that type").

The TypeError that both Gecko and WebKit throw doesn't come from the JS->C++ conversion, but from converting the strings into URLs in https://fetch.spec.whatwg.org/#dom-request

Comment 4 Deleted

Status: WontFix (was: Available)
Right, thanks! Both "null" and "undefined" are perfectly fine URLs when relative paths are accepted, so I guess Gecko/WebKit shouldn't be throwing at all?

I still find this very unexpected behaviour, but if it's correct per WebIDL then this isn't a Chromium issue. Let's mark it as such.
What version of firefox were you seeing it throw a TypeError?  I just checked in the console on FF59 and last nightly.  Both produce a relative URL for me.
My Firefox ESR installation seems to be stuck on FF52, so that figures :)
I've just tried FF62 beta. fetch(null) throws when I'm on "about:blank", and returns a pending promise if I call it from a regular page (e.g. https://mozilla.org)

Sign in to add a comment