Skip to content
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.

Sign up for GitHub

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

Uploading a Request made from a ReadableStream body #425

Merged
merged 6 commits into from
Jan 17, 2017
Merged

Uploading a Request made from a ReadableStream body #425

Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
0cef38e
Add upload streams (a Request with a ReadableStream body)
yutakahirano Nov 22, 2016
5fc6fb7
fix
yutakahirano Jan 6, 2017
33d66ff
fix
yutakahirano Jan 6, 2017
e0ab785
fix
yutakahirano Jan 6, 2017
f09599a
fix
yutakahirano Jan 10, 2017
19cec5f
fix
yutakahirano Jan 11, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
92 changes: 74 additions & 18 deletions fetch.bs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ Markup Shorthands: css off
!Commits: [SNAPSHOT-LINK]
!Commits: <a href=https://twitter.com/fetchstandard>@fetchstandard</a>
!Translation (non-normative): <span title=Japanese><a href=https://triple-underscore.github.io/Fetch-ja.html lang=ja hreflang=ja rel=alternate>日本語</a></span>
Translate IDs: typedefdef-bodyinit bodyinit,typedefdef-responsebodyinit responsebodyinit,dictdef-requestinit requestinit,typedefdef-requestinfo requestinfo,enumdef-requesttype requesttype,enumdef-requestdestination requestdestination,enumdef-requestmode requestmode,enumdef-requestcredentials requestcredentials,enumdef-requestcache requestcache,enumdef-requestredirect requestredirect,dictdef-responseinit responseinit,enumdef-responsetype responsetype
Translate IDs: typedefdef-bodyinit bodyinit,dictdef-requestinit requestinit,typedefdef-requestinfo requestinfo,enumdef-requesttype requesttype,enumdef-requestdestination requestdestination,enumdef-requestmode requestmode,enumdef-requestcredentials requestcredentials,enumdef-requestcache requestcache,enumdef-requestredirect requestredirect,dictdef-responseinit responseinit,enumdef-responsetype responsetype
</pre>

<script src=https://resources.whatwg.org/file-issue.js async></script>
Expand Down Expand Up @@ -571,13 +571,16 @@ user-agent-defined <a for=header>value</a> for the
<p>A <dfn export id=concept-body>body</dfn> consists of:

<ul>
<li><p>A <dfn export for=body id=concept-body-stream>stream</dfn> (a {{ReadableStream}} object).
<li><p>A <dfn export for=body id=concept-body-stream>stream</dfn> (null or a {{ReadableStream}}
object).

<li><p>A <dfn export for=body id=concept-body-transmitted>transmitted bytes</dfn>
(an integer), initially 0.

<li><p>A <dfn export for=body id=concept-body-total-bytes>total bytes</dfn> (an
integer), initially 0.

<li><p>A <dfn export for=body id=concept-body-source>source</dfn>, initially null.
</ul>

<p>A <a for=/>body</a> <var>body</var> is said to be
Expand Down Expand Up @@ -2984,6 +2987,10 @@ in addition to <a>HTTP fetch</a> above.
<a lt="include credential">includes credentials</a>, then return a
<a>network error</a>.

<li><p>If <var>actualResponse</var>'s <a for=response>status</a> is not <code>303</code>,
<var>request</var>'s <a for=request>body</a> is non-null, and <var>request</var>'s
<a for=request>body</a>'s <a for=body>source</a> is null, then return a <a>network error</a>.

<li>
<p>If <i>CORS flag</i> is set and <var>actualResponse</var>'s
<a for=response>location URL</a>
Expand All @@ -3008,6 +3015,15 @@ in addition to <a>HTTP fetch</a> above.
to `<code>GET</code>` and <var>request</var>'s <a for=request>body</a>
to null.

<li>
<p>If <var>request</var>'s <a for=request>body</a> is non-null, then set <var>request</var>'s
<a for=request>body</a> to the first part of <a lt=extract for=BodyInit>extracting</a>
<var>request</var>'s <a for=request>body</a>'s <a for=body>source</a>.

<p class="note no-backref"><var>request</var>'s <a for=request>body</a>'s <a for=body>source</a>'s
nullity has already been checked. The <a lt=extract for=BodyInit>extracting</a> operation cannot
throw as it was called for the same <a for=body>source</a> before.

<li><p>Append <var>actualResponse</var>'s
<a for=response>location URL</a> to <var>request</var>'s
<a for=request>url list</a>.
Expand Down Expand Up @@ -3035,17 +3051,31 @@ steps:
<i>authentication-fetch flag</i>.

<ol>
<li><p>Let <var>httpRequest</var> be null.

<li><p>If <var>request</var>'s <a for=request>window</a> is "<code>no-window</code>" and
<var>request</var>'s <a for=request>redirect mode</a> is "<code>error</code>", then set
<var>httpRequest</var> to <var>request</var>.

<li>
<p>Let <var>httpRequest</var> be <var>request</var> if
<var>request</var>'s <a for=request>window</a> is "<code>no-window</code>"
and <var>request</var>'s <a for=request>redirect mode</a> is
"<code>error</code>", and the result of <a lt=clone for=request>cloning</a>
<var>request</var> otherwise.
<p>Otherwise, run these substeps:

<p class="note no-backref">A <var>request</var> is typically cloned as it needs to be possible to
add <a>headers</a> and read its
<a for=request>body</a> without affecting <var>request</var>. As
<var>request</var> is reused with redirects, authentication, and proxy authentication.
<ol>
<li><p>Set <var>httpRequest</var> to a copy of <var>request</var> except for its
<a for=request>body</a>.

<li><p>Let <var>body</var> be <var>request</var>'s <a for=request>body</a>.

<li><p>Set <var>httpRequest</var>'s <a for=request>body</a> to <var>body</var>.

<li><p>If <var>body</var> is non-null, then set <var>request</var>'s <a for=request>body</a> to a
new <a for=/>body</a> whose <a for=body>stream</a> is null and whose <a for=body>source</a> is
<var>body</var>'s <a for=body>source</a>.
</ol>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This still seems wrong. Shouldn't we be setting httpRequest's body rather than that of request? Also, what happens to request's body?

The note should probably also move to after the </ol> closing tag. We copy and move body around to preserve memory consumption. And we copy in the first place since it needs to be possible to add headers without affecting request, as request is reused with redirects et al. It seems you dropped part of that note, but we should keep that.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, sorry. Fixed. The intention is, passing the ReadableStream to httpRequests for sending the payload and keeping the source information in request in order to restore the payload if necessary.


<p class="note no-backref">Here we do not <a for=request>clone</a> <var>request</var> in order
to reduce memory consumption. <var>request</var> can be reused with redirects, authentication,
and proxy authentication.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This note is still problematic I think compared to the original. I think it should say something like the following:

A request is copied as httpRequest here as we need to be able to add headers to httpRequest and read its body without affecting request. Namely, request can be reused with redirects, authentication, and proxy authentication. We copy rather than clone in order to reduce memory consumption. In case request's body's source is a ReadableStream object, redirects and authentication will end up failing the fetch.

Does that make sense to you?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd say "request's body's source is null" or "request's body is made from ReadableStream" insetad of "request's body's source is a ReadableStream object".

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, yes, that makes sense.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fetch post-call Always return 415 ？？？？？？？？？？WHY


<li>
<p>Let <i>credentials flag</i> be set if one of
Expand Down Expand Up @@ -3326,6 +3356,22 @@ steps:
<li class=XXX><p>Needs testing: multiple `<code>WWW-Authenticate</code>` headers, missing,
parsing issues.

<li>
<p>If <var>request</var>'s <a for=request>body</a> is non-null, then run these subsubsteps:

<ol>
<li><p>If <var>request</var>'s <a for=request>body</a>'s <a for=body>source</a> is null,
then return a <a>network error</a>.

<li>
<p>Set <var>request</var>'s <a for=request>body</a> to the first part of
<a lt=extract for=BodyInit>extracting</a> <var>request</var>'s <a for=request>body</a>'s
<a for=body>source</a>.

<p class="note no-backref">The <a lt=extract for=BodyInit>extracting</a> operation cannot
throw as it was called for the same <a for=body>source</a> before.
</ol>

<li>
<p>If <var>request</var>'s
<a for=request>use-URL-credentials flag</a> is unset or
Expand Down Expand Up @@ -4184,9 +4230,7 @@ method, when invoked, must run these steps:
<h3 id=body-mixin>Body mixin</h3>

<pre class=idl>
typedef (Blob or BufferSource or FormData or URLSearchParams or USVString) BodyInit;

typedef (BodyInit or ReadableStream) ResponseBodyInit;</pre>
typedef (Blob or BufferSource or FormData or URLSearchParams or ReadableStream or USVString) BodyInit;</pre>

<p>To <dfn id=concept-bodyinit-extract for=BodyInit export>extract</dfn> a <a for=/>body</a> and a
`<code>Content-Type</code>` <a for=header>value</a> from
Expand All @@ -4199,7 +4243,9 @@ typedef (BodyInit or ReadableStream) ResponseBodyInit;</pre>

<li><p>Let <var>Content-Type</var> be null.

<li><p>Let <var>action</var> be null.
<li><p>Let <var>action</var> be null.

<li><p>Let <var>source</var> be null.

<li>
<p>Switch on <var>object</var>'s type:
Expand All @@ -4212,6 +4258,8 @@ typedef (BodyInit or ReadableStream) ResponseBodyInit;</pre>
<p>If <var>object</var>'s {{Blob/type}}
attribute is not the empty byte sequence, set <var>Content-Type</var> to its value.

<p>Set <var>source</var> to <var>object</var>.

<dt><code>BufferSource</code>
<dd>
<p><a for=ReadableStream>Enqueue</a> a <code>Uint8Array</code> object
Expand All @@ -4220,6 +4268,8 @@ typedef (BodyInit or ReadableStream) ResponseBodyInit;</pre>
<var>stream</var>. If that threw an exception,
<a abstract-op>error</a> <var>stream</var> with that exception.

<p>Set <var>source</var> to <var>object</var>.

<dt>{{FormData}}
<dd>
<p>Set <var>action</var> to an action that runs the
Expand All @@ -4232,6 +4282,8 @@ typedef (BodyInit or ReadableStream) ResponseBodyInit;</pre>
<a><code>multipart/form-data</code> boundary string</a> generated by the
<a><code>multipart/form-data</code> encoding algorithm</a>.

<p>Set <var>source</var> to <var>object</var>.

<dt>{{URLSearchParams}}
<dd>
<p>Set <var>action</var> to an action that runs the
Expand All @@ -4243,12 +4295,16 @@ typedef (BodyInit or ReadableStream) ResponseBodyInit;</pre>
<p>Set <var>Content-Type</var> to
`<code>application/x-www-form-urlencoded;charset=UTF-8</code>`.

<p>Set <var>source</var> to <var>object</var>.

<dt><code>USVString</code>
<dd>
<p>Set <var>action</var> to an action that runs <a>UTF-8 encode</a> on <var>object</var>.

<p>Set <var>Content-Type</var> to `<code>text/plain;charset=UTF-8</code>`.

<p>Set <var>source</var> to <var>object</var>.

<dt>{{ReadableStream}}
<dd><p>Set <var>stream</var> to <var>object</var>.
</dl>
Expand All @@ -4272,8 +4328,8 @@ typedef (BodyInit or ReadableStream) ResponseBodyInit;</pre>
<a abstract-op>close</a> <var>stream</var>.
</ol>

<li><p>Let <var>body</var> be a <a for=/>body</a> whose
<a for=body>stream</a> is <var>stream</var>.
<li><p>Let <var>body</var> be a <a for=/>body</a> whose <a for=body>stream</a> is
<var>stream</var> and whose <a for=body>source</a> is <var>source</var>.

<li><p>Return <var>body</var> and <var>Content-Type</var>.
</ol>
Expand Down Expand Up @@ -4914,7 +4970,7 @@ run these steps:

<h3 id=response-class>Response class</h3>

<pre class=idl>[Constructor(optional ResponseBodyInit? body = null, optional ResponseInit init),
<pre class=idl>[Constructor(optional BodyInit? body = null, optional ResponseInit init),
Exposed=(Window,Worker)]
interface Response {
[NewObject] static Response error();
Expand Down