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

Jump to bottom

Remove HTTP_PUSH_ALREADY_IN_CACHE error #2813

Merged
merged 3 commits into from Jun 27, 2019
Merged

Remove HTTP_PUSH_ALREADY_IN_CACHE error #2813

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
017b22b
Remove HTTP_PUSH_ALREADY_IN_CACHE error
LPardue Jun 19, 2019
e246182
Apply Kazuho's suggestion to replace HTTP_PUSH_REFUSED with HTTP_REQU…
LPardue Jun 19, 2019
8af1c75
Fix broken grammar
LPardue Jun 20, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
22 changes: 10 additions & 12 deletions draft-ietf-quic-http.md
View file
Open in desktop
Expand Up @@ -785,10 +785,9 @@ amount of data a server may commit to the pushed stream.

If a promised server push is not needed by the client, the client SHOULD send a
CANCEL_PUSH frame. If the push stream is already open or opens after sending the
CANCEL_PUSH frame, a QUIC STOP_SENDING frame with an appropriate error code can
Copy link
Member

Choose a reason for hiding this comment

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

To me the subject and the verb being omitted in this sentence was a bit surprising, but that might be due to me not being a native speaker.

Copy link
Member Author

Choose a reason for hiding this comment

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

It is an error, thanks for picking it up. I would convert this to

"a QUIC STOP_SENDING frame with an error code of HTTP_REQUEST_CANCELLED can be used."

Alternatively, elsewhere we talk about "triggering QUIC STOP_SENDING" which is under review as part of @MikeBishop #2805

also be used (e.g., HTTP_PUSH_REFUSED, HTTP_PUSH_ALREADY_IN_CACHE; see
{{errors}}). This asks the server not to transfer additional data and indicates
that it will be discarded upon receipt.
CANCEL_PUSH frame, a QUIC STOP_SENDING frame with an error code of
HTTP_REQUEST_CANCELLED can be used. This asks the server not to transfer
additional data and indicates that it will be discarded upon receipt.

# Connection Closure

Expand Down Expand Up @@ -1599,18 +1598,17 @@ HTTP_WRONG_SETTING_DIRECTION (0x01):
: A client-only setting was sent by a server, or a server-only setting by a
client.

HTTP_PUSH_REFUSED (0x02):
: The server has attempted to push content which the client will not accept
on this connection.
Reserved (0x02):
: This code is reserved and has no meaning.

HTTP_INTERNAL_ERROR (0x03):
: An internal error has occurred in the HTTP stack.

HTTP_PUSH_ALREADY_IN_CACHE (0x04):
: The server has attempted to push content which the client has cached.
Reserved (0x04):
: This code is reserved and has no meaning.

HTTP_REQUEST_CANCELLED (0x05):
: The request or its response is cancelled.
: The request or its response (including pushed response) is cancelled.

HTTP_INCOMPLETE_REQUEST (0x06):
: The client's stream terminated without containing a fully-formed request.
Expand Down Expand Up @@ -1903,9 +1901,9 @@ The entries in the following table are registered by this document.
| ----------------------------------- | ---------- | ---------------------------------------- | ---------------------- |
| HTTP_NO_ERROR | 0x0000 | No error | {{http-error-codes}} |
| HTTP_WRONG_SETTING_DIRECTION | 0x0001 | Setting sent in wrong direction | {{http-error-codes}} |
| HTTP_PUSH_REFUSED | 0x0002 | Client refused pushed content | {{http-error-codes}} |
| Reserved | 0x0002 | N/A | N/A |
Copy link
Member

Choose a reason for hiding this comment

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

My sense is that we want to do a final cleanup for reserved error codes. We might reserve some for keeping the relationship with RFC 7540 clean, but I don't think that a renumbering is a bad idea after all this work on fixing errors completes.

Copy link
Member Author

Choose a reason for hiding this comment

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

Definitely. Not many good reasons exist to reserve codes for the shipped RFC.

| HTTP_INTERNAL_ERROR | 0x0003 | Internal error | {{http-error-codes}} |
| HTTP_PUSH_ALREADY_IN_CACHE | 0x0004 | Pushed content already cached | {{http-error-codes}} |
| Reserved | 0x0004 | N/A | N/A |
| HTTP_REQUEST_CANCELLED | 0x0005 | Data no longer needed | {{http-error-codes}} |
| HTTP_INCOMPLETE_REQUEST | 0x0006 | Stream terminated early | {{http-error-codes}} |
| HTTP_CONNECT_ERROR | 0x0007 | TCP reset or error on CONNECT request | {{http-error-codes}} |
Expand Down