Skip to content

Specify "upload body of a request" #449

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

Merged
merged 5 commits into from
Jan 6, 2017
Merged

Specify "upload body of a request" #449

Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5f5fce9
initial
yutakahirano Jan 5, 2017
53ee078
fix
yutakahirano Jan 5, 2017
f83170a
fix
yutakahirano Jan 6, 2017
23e2d2e
add some breathing room
annevk Jan 6, 2017
8e193fd
Minor style issues
domenic Jan 6, 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
56 changes: 42 additions & 14 deletions fetch.bs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,18 +213,6 @@ of:
<a>queue a fetch task</a> on <var>request</var> to <a>process request end-of-body</a>
for <var>request</var>.

<hr>

<p>To <dfn>read a <var>request</var></dfn>, if <var>request</var>'s
<a for=request>body</a> is non-null, whenever
<var>request</var>'s <a for=request>body</a> is read from (i.e.
is transmitted or read by script), increase <var>request</var>'s
<a for=request>body</a>'s
<a for=body>transmitted bytes</a> with the amount of payload body
bytes transmitted and then <a>queue a fetch task</a> on <var>request</var> to
<a>process request body</a> for <var>request</var>.
<!-- XXX xref "read", "payload body" -->


<h3 id=http>HTTP</h3>

Expand Down Expand Up @@ -1121,6 +1109,46 @@ or "<code>worker</code>".
<li><p>Return <var>newRequest</var>.
</ol>

<p>To <dfn export for=request id=concept-request-upload-body>upload body</dfn> of a
Copy link
Member

Choose a reason for hiding this comment

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

Nit: I would phrase this as "upload a body for a request".

Copy link
Member

Choose a reason for hiding this comment

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

I think we should avoid "upload" entirely. The protocol specification doesn't use it either and it doesn't make much sense for non-HTTP URLs (although I guess those will fail to function anyway).

Copy link
Member Author

@yutakahirano yutakahirano Jan 6, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Then which term should we use? In other words, what are the "primitive" operations on the network layer which I can use?

Copy link
Member

Choose a reason for hiding this comment

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

What was wrong with read?

Copy link
Member

Choose a reason for hiding this comment

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

If read cannot be used, "transmit" would be okay.

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.

<a for=/>request</a> <var>request</var>, run these steps:

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

<li><p>If <var>body</var> is null, then <a>queue a fetch task</a> on <var>request</var> to
<a>process request end-of-body</a> for <var>request</var> and abort these steps.

<li>
<p>Let <var>read</var> be the result of <a lt="read a chunk" for=ReadableStream>reading a
chunk</a> from <var>body</var>'s <a for=body>stream</a>.

<ul>
<li>
<p>When <var>read</var> is fulfilled with an object whose <code>done</code>
property is false and whose <code>value</code> property is a
<code>Uint8Array</code> object, run these substeps:
Copy link
Member

Choose a reason for hiding this comment

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

Is this the right way of phrasing this, @domenic?

Copy link
Member

Choose a reason for hiding this comment

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

It seems solid for a web spec. ES-style specs could use more formal internal slot inspection but that doesn't seem necessary here.

Copy link
Member

Choose a reason for hiding this comment

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

I guess we can make it formal once IDL formalizes promises.

Copy link
Member

Choose a reason for hiding this comment

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

Oh, I see, I was focusing on the Uint8Array issue, but the "object whose done property is false" part is the imprecise part. In particular, what if obj.done or obj.value is a throwing getter? Let me push a commit to fix.

Copy link
Member

Choose a reason for hiding this comment

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

Nevermind, these objects are created by the streams, and can never have getters or non-booleans or similar. The only "bad" thing that can happen is value not being a Uint8Array, and we have that covered.


<ol>
<li><p>Let <var>bytes</var> be the bytes represented by the <code>Uint8Array</code> object.

<li><p>Upload <var>bytes</var>.

<li><p>Increase <var>body</var>'s <a for=body>transmitted bytes</a> with <var>bytes</var>'
length.

<li><p>Run the above step again.
</ol>

<li><p>When <var>read</var> is fulfilled with an object whose <code>done</code>
property is true, <a>queue a fetch task</a> on <var>request</var> to
<a>process request end-of-body</a> for <var>request</var>.

<li><p>When <var>read</var> is fulfilled with a value that matches with neither of the
above patterns, or <var>read</var> is rejected, <a lt=terminated for=fetch>terminate</a> the
ongoing fetch with reason <i>fatal</i>.
Copy link
Member

Choose a reason for hiding this comment

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

This seems a little weird since we don't have a handle on the ongoing fetch and it's not passed to this algorithm. Perhaps it's okay for now though, but we should maybe file a follow up issue to see if we can fix that somehow.

</ul>
</ol>


<h4 id=responses>Responses</h4>

Expand Down Expand Up @@ -2791,7 +2819,7 @@ optional <i>CORS flag</i> and <i>CORS-preflight flag</i>, run these steps:
<p>If <var>response</var> is not null, then run these substeps:

<ol>
<li><p><a lt="Read a request">Read <var>request</var></a>.
<li><p><a for=request>Upload body</a> of <var>request</var>.

<li><p>Set <var>actualResponse</var> to <var>response</var>, if <var>response</var> is not a
<a>filtered response</a>, and to <var>response</var>'s
Expand Down Expand Up @@ -3457,7 +3485,7 @@ steps:
"<code>deprecated</code>" state value ought to be a temporary and last resort kind
of option.

<p><a lt="Read a request">Read <var>request</var></a>.
<p><a for=request>Upload body</a> of <var>request</var>.

<li>
<p>Let <var>strategy</var> be an object. The user agent may choose any object.
Expand Down