-
Notifications
You must be signed in to change notification settings - Fork 55
Add headers constructor option and readonly response wt.headers attribute #713
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -727,6 +727,7 @@ interface WebTransport { | |||||
| readonly attribute WebTransportCongestionControl congestionControl; | ||||||
| [EnforceRange] attribute unsigned short? anticipatedConcurrentIncomingUnidirectionalStreams; | ||||||
| [EnforceRange] attribute unsigned short? anticipatedConcurrentIncomingBidirectionalStreams; | ||||||
| [SameObject] readonly attribute Headers? headers; | ||||||
| readonly attribute DOMString protocol; | ||||||
|
|
||||||
| readonly attribute Promise<WebTransportCloseInfo> closed; | ||||||
|
|
@@ -821,6 +822,10 @@ A {{WebTransport}} object has the following internal slots. | |||||
| <td class="non-normative">The number of concurrently open | ||||||
| [=bidirectional=] streams the application anticipates the server creating, or null. | ||||||
| </tr> | ||||||
| <tr> | ||||||
| <td><dfn>`[[Headers]]`</dfn> | ||||||
| <td class="non-normative">A {{Headers}} object or null. Initially null. | ||||||
| </tr> | ||||||
| <tr> | ||||||
| <td><dfn>`[[Protocol]]`</dfn> | ||||||
| <td class="non-normative">A string indicating the application-level protocol selected by the server, | ||||||
|
|
@@ -924,6 +929,8 @@ agent MUST run the following steps: | |||||
| :: |anticipatedConcurrentIncomingUnidirectionalStreams| | ||||||
| : {{[[AnticipatedConcurrentIncomingBidirectionalStreams]]}} | ||||||
| :: |anticipatedConcurrentIncomingBidirectionalStreams| | ||||||
| : {{[[Headers]]}} | ||||||
| :: null | ||||||
| : {{[[Protocol]]}} | ||||||
| :: an empty string | ||||||
| : {{[[Closed]]}} | ||||||
|
|
@@ -980,6 +987,13 @@ agent MUST run the following steps: | |||||
|
|
||||||
| 1. Set |request|'s [=request/method=] to "`CONNECT`", and set the method's associated | ||||||
| `:protocol` <dfn>pseudo-header</dfn> to `"webtransport"`. | ||||||
| 1. Let |headers| be {{WebTransport/constructor(url, options)/options}}["{{WebTransportOptions/headers}}"]. | ||||||
| 1. If |headers| is a not a {{Headers}} object, then set |headers| to a new {{Headers}} | ||||||
| object [=Headers/filled=] with | ||||||
| {{WebTransport/constructor(url, options)/options}}["{{WebTransportOptions/headers}}"]. | ||||||
| 1. Set |request|'s [=request/header list=] to a copy of |headers|'s [=Headers/header list=]. | ||||||
| 1. If |request|'s [=request/header list=] [=header list/contains=] `WT-Available-Protocols`, then | ||||||
| throw a {{TypeError}}. | ||||||
| 1. If |protocols| is not empty, [=header list/set a structured field value=] with | ||||||
| (`WT-Available-Protocols`, a | ||||||
| <a href="https://html.spec.whatwg.org/#http-structured-header-list"> | ||||||
|
|
@@ -989,7 +1003,7 @@ agent MUST run the following steps: | |||||
| [=header list=]. | ||||||
| 1. [=Fetch=] |request|, with [=useParallelQueue=] set to true, and [=processResponse=] | ||||||
| set to the following steps given a |response|: | ||||||
| 1. [=Process a WebTransport fetch response=] with |response|, |origin|, |protocols|, and |congestionControl|. | ||||||
| 1. [=Process a WebTransport fetch response=] with |response| and |transport|. | ||||||
| 1. Return |transport|. | ||||||
|
|
||||||
| </div> | ||||||
|
|
@@ -1035,7 +1049,7 @@ To <dfn export>obtain a WebTransport connection</dfn>, given a [=network partiti | |||||
| </div> | ||||||
|
|
||||||
| <div algorithm="process a WebTransport fetch response"> | ||||||
| To <dfn>process a WebTransport fetch response</dfn>, given a |response|, and |congestionControl|, run these steps: | ||||||
| To <dfn>process a WebTransport fetch response</dfn>, given a |response| and |transport|, run these steps: | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: consider specifying the types of the arguments here. |
||||||
| 1. If |response| is [=network error=], then abort the remaining steps and [=queue a network task=] with | ||||||
| |transport| to run these steps: | ||||||
| 1. If |transport|.{{[[State]]}} is `"closed"` or `"failed"`, then abort these steps. | ||||||
|
|
@@ -1059,14 +1073,18 @@ To <dfn>process a WebTransport fetch response</dfn>, given a |response|, and |co | |||||
| {{WebTransportErrorOptions/source}} is `"session"`. | ||||||
| 1. [=Cleanup=] |transport| with |error|. | ||||||
| 1. If the user agent supports more than one congestion control algorithm, choose one | ||||||
| appropriate for |congestionControl| for sending of data on this |connection|. | ||||||
| appropriate for |transport|'s {{[[CongestionControl]]}} for sending of data on this |connection|. | ||||||
| 1. [=Queue a network task=] with |transport| to run these steps: | ||||||
| 1. Assert: [=this=]'s {{[[Datagrams]]}}'s {{[[OutgoingMaxDatagramSize]]}} is an integer. | ||||||
| 1. If |transport|.{{[[State]]}} is not `"connecting"`: | ||||||
| 1. [=In parallel=], [=session/terminate=] |session|. | ||||||
| 1. Abort these steps. | ||||||
| 1. Set |transport|.{{[[State]]}} to `"connected"`. | ||||||
| 1. Set |transport|.{{[[Session]]}} to |session|. | ||||||
| 1. Let |headers| be a copy of |response|’s [=response/header list=]. | ||||||
| 1. Remove from |headers| each header whose name is a [=byte-case-insensitive=] match for `WT-Protocol`. | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||||||
| 1. Set |transport|.{{[[Headers]]}} to a new {{Headers}} object with |transport|'s [=relevant realm=], | ||||||
| whose [=Headers/header list=] is |headers| and guard is "`immutable`". | ||||||
| 1. Set |transport|.{{[[Protocol]]}} to either the string value of the negotiated application | ||||||
| protocol if present, following [[!WEB-TRANSPORT-OVERVIEW]] | ||||||
| [Section 3.1](https://datatracker.ietf.org/doc/html/draft-ietf-webtrans-overview-09/#section-3.1), | ||||||
|
|
@@ -1199,6 +1217,11 @@ Note: Setting {{WebTransport/anticipatedConcurrentIncomingUnidirectionalStreams} | |||||
| {{WebTransport/anticipatedConcurrentIncomingBidirectionalStreams}} does not guarantee | ||||||
| the application will receive the number of streams it anticipates. | ||||||
|
|
||||||
| : <dfn for="WebTransport" attribute>headers</dfn> | ||||||
| :: Once a [=WebTransport session=] has been established, holds the application-level | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
? |
||||||
| headers set by the server, if any. Initially null. | ||||||
| The getter steps are to return [=this=]'s {{[[Headers]]}}. | ||||||
|
Comment on lines
+1221
to
+1223
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would it be clearer if we added some text indicating that these headers are subject to the forbidden request-header list as defined by https://fetch.spec.whatwg.org/#methods section 2.2.2?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a little hand-wavy. Shouldn't you define somewhere a slot that holds null or a Headers object?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @annevk isn't that https://pr-preview.s3.amazonaws.com/jan-ivar/webtransport/pull/713.html#dom-webtransport-headers-slot? Or did you mean something else?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh sorry, I missed [[ and ]] and thought this was referring to the object. |
||||||
|
|
||||||
| : <dfn for="WebTransport" attribute>protocol</dfn> | ||||||
| :: Once a [=WebTransport session=] has been established and the {{protocols}} | ||||||
| constructor option was used to provide a non-empty array, returns the application-level protocol | ||||||
|
|
@@ -1530,6 +1553,7 @@ dictionary WebTransportHash { | |||||
| dictionary WebTransportOptions { | ||||||
| boolean allowPooling = false; | ||||||
| boolean requireUnreliable = false; | ||||||
| HeadersInit headers = {}; | ||||||
| sequence<WebTransportHash> serverCertificateHashes = []; | ||||||
| WebTransportCongestionControl congestionControl = "default"; | ||||||
| [EnforceRange] unsigned short? anticipatedConcurrentIncomingUnidirectionalStreams = null; | ||||||
|
|
@@ -1558,6 +1582,11 @@ that determine how the [=WebTransport session=] is established and used. | |||||
| :: When set to true, the [=WebTransport session=] cannot be established over an | ||||||
| HTTP/2 [=connection=] if an HTTP/3 [=connection=] is not possible. | ||||||
|
|
||||||
| : <dfn for="WebTransportOptions" dict-member>headers</dfn> | ||||||
| :: Optionally, a {{Headers}} object, an object literal, or an array of two-item arrays | ||||||
| to set the [=request=] [=request/header list=] to use when [=session/establishing=] | ||||||
| a [=WebTransport session=]. | ||||||
|
|
||||||
| : <dfn for="WebTransportOptions" dict-member>serverCertificateHashes</dfn> | ||||||
| :: This option is only supported for transports using dedicated connections. | ||||||
| For transport protocols that do not support this feature, having this | ||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems you are not validating the
Headersobject and just copy all its headers straight onto request. Are you sure that's what you want? @wilaw's comment below suggests you are expecting some kind of validation at least.This is why the
Requestconstructor for instance creates its own instance always and then attempts to forward the headers (which might end up throwing).Perhaps we could/should create some kind of abstraction for this which can be shared by Fetch and WebTransport.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch! We definitely want filtering by the forbidden request header list