Skip to content

Commit

Permalink
feat(fetch): add Request{Init}.duplex and add WPTs (#1681)
Browse files Browse the repository at this point in the history
* feat(fetch): add `Request.duplex` and add WPTs

* feat: add remaining applicable tests
  • Loading branch information
KhafraDev committed Oct 4, 2022
1 parent 0964a83 commit 733b3a0
Show file tree
Hide file tree
Showing 18 changed files with 936 additions and 64 deletions.
31 changes: 28 additions & 3 deletions lib/fetch/request.js
Expand Up @@ -472,15 +472,21 @@ class Request {
// 38. If inputOrInitBody is non-null and inputOrInitBody’s source is
// null, then:
if (inputOrInitBody != null && inputOrInitBody.source == null) {
// 1. If this’s request’s mode is neither "same-origin" nor "cors",
// 1. If initBody is non-null and init["duplex"] does not exist,
// then throw a TypeError.
if (initBody != null && init.duplex == null) {
throw new TypeError('RequestInit: duplex option is required when sending a body.')
}

// 2. If this’s request’s mode is neither "same-origin" nor "cors",
// then throw a TypeError.
if (request.mode !== 'same-origin' && request.mode !== 'cors') {
throw new TypeError(
'If request is made from ReadableStream, mode should be "same-origin" or "cors"'
)
}

// 2. Set this’s request’s use-CORS-preflight flag.
// 3. Set this’s request’s use-CORS-preflight flag.
request.useCORSPreflightFlag = true
}

Expand Down Expand Up @@ -821,7 +827,17 @@ Object.defineProperties(Request.prototype, {
headers: kEnumerableProperty,
redirect: kEnumerableProperty,
clone: kEnumerableProperty,
signal: kEnumerableProperty
signal: kEnumerableProperty,
duplex: {
...kEnumerableProperty,
get () {
// The duplex getter steps are to return "half".
return 'half'
},
set () {

}
}
})

webidl.converters.Request = webidl.interfaceConverter(
Expand Down Expand Up @@ -929,6 +945,15 @@ webidl.converters.RequestInit = webidl.dictionaryConverter([
{
key: 'window',
converter: webidl.converters.any
},
{
key: 'duplex',
converter: webidl.converters.DOMString,
allowedValues: ['half'],
// TODO(@KhafraDev): this behavior is incorrect, but
// without it, a WPT throws with an uncaught exception,
// causing the entire WPT runner to crash.
defaultValue: 'half'
}
])

Expand Down
48 changes: 0 additions & 48 deletions test/fetch/abort.js
Expand Up @@ -4,7 +4,6 @@ const { test } = require('tap')
const { fetch } = require('../..')
const { createServer } = require('http')
const { once } = require('events')
const { ReadableStream } = require('stream/web')
const { DOMException } = require('../../lib/fetch/constants')

const { AbortController: NPMAbortController } = require('abort-controller')
Expand Down Expand Up @@ -62,53 +61,6 @@ test('parallel fetch with the same AbortController works as expected', async (t)
t.end()
})

// https://github.com/web-platform-tests/wpt/blob/fd8aeb1bb2eb33bc43f8a5bbc682b0cff6075dfe/fetch/api/abort/general.any.js#L474-L507
test('Readable stream synchronously cancels with AbortError if aborted before reading', async (t) => {
const server = createServer((req, res) => {
res.write('')
res.end()
}).listen(0)

t.teardown(server.close.bind(server))
await once(server, 'listening')

const controller = new AbortController()
const signal = controller.signal
controller.abort()

let cancelReason

const body = new ReadableStream({
pull (controller) {
controller.enqueue(new Uint8Array([42]))
},
cancel (reason) {
cancelReason = reason
}
})

const fetchPromise = fetch(`http://localhost:${server.address().port}`, {
body,
signal,
method: 'POST',
headers: {
'Content-Type': 'text/plain'
}
})

t.ok(cancelReason, 'Cancel called sync')
t.equal(cancelReason.constructor, DOMException)
t.equal(cancelReason.name, 'AbortError')

await t.rejects(fetchPromise, { name: 'AbortError' })

const fetchErr = await fetchPromise.catch(e => e)

t.equal(cancelReason, fetchErr, 'Fetch rejects with same error instance')

t.end()
})

test('Allow the usage of custom implementation of AbortController', async (t) => {
const body = {
fixes: 1605
Expand Down
7 changes: 5 additions & 2 deletions test/types/fetch.test-d.ts
@@ -1,7 +1,7 @@
import { URL } from 'url'
import { Blob } from 'buffer'
import { ReadableStream } from 'stream/web'
import { expectType, expectError } from 'tsd'
import { expectType, expectError, expectAssignable, expectNotAssignable } from 'tsd'
import {
Agent,
BodyInit,
Expand All @@ -10,7 +10,6 @@ import {
Headers,
HeadersInit,
SpecIterableIterator,
SpecIterator,
Request,
RequestCache,
RequestCredentials,
Expand Down Expand Up @@ -166,3 +165,7 @@ expectType<Promise<FormData>>(response.formData())
expectType<Promise<unknown>>(response.json())
expectType<Promise<string>>(response.text())
expectType<Response>(response.clone())

expectType<Request>(new Request('https://example.com', { body: 'Hello, world', duplex: 'half' }))
expectAssignable<RequestInit>({ duplex: 'half' })
expectNotAssignable<RequestInit>({ duplex: 'not valid' })
10 changes: 6 additions & 4 deletions test/wpt/runner/runner/runner.mjs
@@ -1,6 +1,6 @@
import { EventEmitter, once } from 'node:events'
import { readdirSync, readFileSync, statSync } from 'node:fs'
import { isAbsolute, join, resolve } from 'node:path'
import { basename, isAbsolute, join, resolve } from 'node:path'
import { fileURLToPath } from 'node:url'
import { Worker } from 'node:worker_threads'
import { parseMeta } from './util.mjs'
Expand Down Expand Up @@ -93,7 +93,7 @@ export class WPTRunner extends EventEmitter {

worker.on('message', (message) => {
if (message.type === 'result') {
this.handleIndividualTestCompletion(message)
this.handleIndividualTestCompletion(message, basename(test))
} else if (message.type === 'completion') {
this.handleTestCompletion(worker)
}
Expand All @@ -114,14 +114,16 @@ export class WPTRunner extends EventEmitter {
/**
* Called after a test has succeeded or failed.
*/
handleIndividualTestCompletion (message) {
handleIndividualTestCompletion (message, fileName) {
const { fail } = this.#status[fileName] ?? {}

if (message.type === 'result') {
this.#stats.completed += 1

if (message.result.status === 1) {
this.#stats.failed += 1

if (this.#status.fail.includes(message.result.name)) {
if (fail && fail.includes(message.result.name)) {
this.#stats.expectedFailures += 1
} else {
process.exitCode = 1
Expand Down
3 changes: 2 additions & 1 deletion test/wpt/runner/runner/util.mjs
Expand Up @@ -38,8 +38,9 @@ export function parseMeta (fileContents) {
}

switch (groups.type) {
case 'title':
case 'timeout': {
meta.timeout = groups.match
meta[groups.type] = groups.match
break
}
case 'global': {
Expand Down
4 changes: 3 additions & 1 deletion test/wpt/runner/runner/worker.mjs
@@ -1,6 +1,7 @@
import { join } from 'node:path'
import { runInThisContext } from 'node:vm'
import { parentPort, workerData } from 'node:worker_threads'
import { readFileSync } from 'node:fs'
import {
setGlobalOrigin,
Response,
Expand Down Expand Up @@ -65,7 +66,8 @@ runInThisContext(`
globalThis.location = new URL('${url}')
`)

await import('../resources/testharness.cjs')
const harness = readFileSync(join(basePath, '../runner/resources/testharness.cjs'), 'utf-8')
runInThisContext(harness)

// add_*_callback comes from testharness
// stolen from node's wpt test runner
Expand Down
22 changes: 17 additions & 5 deletions test/wpt/status/fetch.status.json
@@ -1,7 +1,19 @@
{
"fail": [
"Stream errors once aborted. Underlying connection closed.",
"Underlying connection is closed when aborting after receiving response - no-cors",
"Already aborted signal rejects immediately"
]
"request-init-stream.any.js": {
"fail": [
"It is error to omit .duplex when the body is a ReadableStream."
]
},
"general.any.js": {
"fail": [
"Stream errors once aborted. Underlying connection closed.",
"Underlying connection is closed when aborting after receiving response - no-cors",
"Already aborted signal rejects immediately"
]
},
"request-disturbed.any.js": {
"fail": [
"Input request used for creating new request became disturbed even if body is not used"
]
}
}
13 changes: 13 additions & 0 deletions test/wpt/tests/fetch/api/request/forbidden-method.any.js
@@ -0,0 +1,13 @@
// META: global=window,worker

// https://fetch.spec.whatwg.org/#forbidden-method
for (const method of [
'CONNECT', 'TRACE', 'TRACK',
'connect', 'trace', 'track'
]) {
test(function() {
assert_throws_js(TypeError,
function() { new Request('./', {method: method}); }
);
}, 'Request() with a forbidden method ' + method + ' must throw.');
}
92 changes: 92 additions & 0 deletions test/wpt/tests/fetch/api/request/request-bad-port.any.js
@@ -0,0 +1,92 @@
// META: global=window,worker

// list of bad ports according to
// https://fetch.spec.whatwg.org/#port-blocking
var BLOCKED_PORTS_LIST = [
1, // tcpmux
7, // echo
9, // discard
11, // systat
13, // daytime
15, // netstat
17, // qotd
19, // chargen
20, // ftp-data
21, // ftp
22, // ssh
23, // telnet
25, // smtp
37, // time
42, // name
43, // nicname
53, // domain
69, // tftp
77, // priv-rjs
79, // finger
87, // ttylink
95, // supdup
101, // hostriame
102, // iso-tsap
103, // gppitnp
104, // acr-nema
109, // pop2
110, // pop3
111, // sunrpc
113, // auth
115, // sftp
117, // uucp-path
119, // nntp
123, // ntp
135, // loc-srv / epmap
137, // netbios-ns
139, // netbios-ssn
143, // imap2
161, // snmp
179, // bgp
389, // ldap
427, // afp (alternate)
465, // smtp (alternate)
512, // print / exec
513, // login
514, // shell
515, // printer
526, // tempo
530, // courier
531, // chat
532, // netnews
540, // uucp
548, // afp
554, // rtsp
556, // remotefs
563, // nntp+ssl
587, // smtp (outgoing)
601, // syslog-conn
636, // ldap+ssl
989, // ftps-data
990, // ftps
993, // ldap+ssl
995, // pop3+ssl
1719, // h323gatestat
1720, // h323hostcall
1723, // pptp
2049, // nfs
3659, // apple-sasl
4045, // lockd
5060, // sip
5061, // sips
6000, // x11
6566, // sane-port
6665, // irc (alternate)
6666, // irc (alternate)
6667, // irc (default)
6668, // irc (alternate)
6669, // irc (alternate)
6697, // irc+tls
10080, // amanda
];

BLOCKED_PORTS_LIST.map(function(a){
promise_test(function(t){
return promise_rejects_js(t, TypeError, fetch("http://example.com:" + a))
}, 'Request on bad port ' + a + ' should throw TypeError.');
});

0 comments on commit 733b3a0

Please sign in to comment.