Bug 226220 - ReadableStream.getReader do not throw a proper exception when parameter is of wrong type
Summary: ReadableStream.getReader do not throw a proper exception when parameter is of...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Bindings (show other bugs)
Version: WebKit Local Build
Hardware: Unspecified Linux
: P2 Normal
Assignee: youenn fablet
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-05-25 06:03 PDT by zyscoder@gmail.com
Modified: 2021-07-01 13:21 PDT (History)
8 users (show)

See Also:


Attachments
Patch (11.36 KB, patch)
2021-07-01 05:30 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
Patch (13.76 KB, patch)
2021-07-01 08:08 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
Patch (14.11 KB, patch)
2021-07-01 08:57 PDT, youenn fablet
ews-feeder: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description zyscoder@gmail.com 2021-05-25 06:03:49 PDT
Steps to reproduce:

(1) Open a tab and navigate to any URL;
(2) Run the following code in the Console of Devtools:
```
b = new Blob();
s = b.stream();
s.getReader(123);
```
(3) Then this code would be evaluated successfully without throwing any exception.

Actual results:

This code is evaluated successfully without throwing any exception.

Expected results:

As https://docs.w3cub.com/dom/readablestream/getreader says, the getReader method accesses an optional parameter containing a property mode, which takes as its value a DOMString specifying the type of reader to create. 
Thus when passing an integer 123 to this API, since 123 is not a dictionary and cannot be converted to a dictionary, an exception should be thrown by Webkit, just like what Chrome and Firefox do.

Evaluating the same code in Chrome and Firefox would throw exceptions:
For Chrome 90.0.4430.212: `VM622:3 Uncaught TypeError: Failed to execute 'getReader' on 'ReadableStream': cannot convert to dictionary.`
For Firefox 89.0b6: `Uncaught TypeError: can't convert 123 to dictionary`
Comment 1 Sam Sneddon [:gsnedders] 2021-05-25 07:15:52 PDT
See also 226215 for another bindings bug with dictionary types.
Comment 2 Chris Dumez 2021-05-25 09:47:49 PDT
This is not using regular IDL in WebKit so I am deferring to Youenn on this one. It seems getReader() is implemented in JS?
Comment 3 Radar WebKit Bug Importer 2021-06-01 06:04:32 PDT
<rdar://problem/78711382>
Comment 4 youenn fablet 2021-07-01 05:30:19 PDT
Created attachment 432680 [details]
Patch
Comment 5 youenn fablet 2021-07-01 08:08:05 PDT
Created attachment 432693 [details]
Patch
Comment 6 Chris Dumez 2021-07-01 08:25:37 PDT
Comment on attachment 432693 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=432693&action=review

> Source/WebCore/Modules/streams/ReadableStream.js:103
> +    // FIXME: We check for @undefined and null to match Chrome and WPT tests, we should check whether we can remove those checks.

This FIXME is wrong. We are doing this change to match the spec and other browser. The current code is wrong, the new code matches the spec and there shouldn't be a FIXME.

Cf. https://heycam.github.io/webidl/#es-dictionary:
`If Type(esDict) is not Undefined, Null or Object, then throw a TypeError.`

Cf. https://streams.spec.whatwg.org/#rs-class (that says it is a dictionary parameter).

If this code was using WebIDL, we'd be getting correct / standard behavior for free.
Comment 7 youenn fablet 2021-07-01 08:57:24 PDT
Created attachment 432702 [details]
Patch
Comment 8 EWS 2021-07-01 10:41:58 PDT
Committed r279472 (239326@main): <https://commits.webkit.org/239326@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 432702 [details].