Style. #1402
Merged
Style. #1402
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mnot
requested changes
Feb 8, 2021
draft-ietf-httpbis-proxy-status.md
Outdated
| @@ -99,7 +99,7 @@ Parameters on each member convey additional information about that intermediary' | |||
|
|
|||
| When adding a value to the Proxy-Status field, intermediaries SHOULD preserve the existing members of the field, to allow debugging of the entire chain of intermediaries handling the request. | |||
|
|
|||
| Proxy-Status MAY be sent in HTTP trailers. For example, if an intermediary is streaming a response and the upstream connection suddenly terminates, Proxy-Status can be appended to the trailers of the outgoing message (since the headers have already been sent). As with all trailers, it might be silently discarded along the path to the user agent, so this SHOULD NOT be done unless it is not possible to send it in headers, and an intermediary MUST NOT send Proxy-Status as a trailer field unless it has also sent a corresponding Proxy-Status header field in the same message, so that the trailer value's ordering relative to other intermediaries is preserved. | |||
| Proxy-Status MAY be sent in HTTP trailers. For example, if an intermediary is streaming a response and the upstream connection suddenly terminates, Proxy-Status can be appended only to the trailers of the outgoing message, since the headers have already been sent. As with all trailers, it might be silently discarded along the path to the user agent, so this SHOULD NOT be done unless it is not possible to send it in headers, and an intermediary MUST NOT send Proxy-Status as a trailer field unless it has also sent a corresponding Proxy-Status header field in the same message, so that the trailer value's ordering relative to other intermediaries is preserved. | |||
There was a problem hiding this comment.
Suggested change
| Proxy-Status MAY be sent in HTTP trailers. For example, if an intermediary is streaming a response and the upstream connection suddenly terminates, Proxy-Status can be appended only to the trailers of the outgoing message, since the headers have already been sent. As with all trailers, it might be silently discarded along the path to the user agent, so this SHOULD NOT be done unless it is not possible to send it in headers, and an intermediary MUST NOT send Proxy-Status as a trailer field unless it has also sent a corresponding Proxy-Status header field in the same message, so that the trailer value's ordering relative to other intermediaries is preserved. | |
| Proxy-Status MAY be sent in HTTP trailers. For example, if an intermediary is streaming a response and the upstream connection suddenly terminates, Proxy-Status can be appended to the trailers of the outgoing message, since the headers have already been sent. As with all trailers, it might be silently discarded along the path to the user agent, so this SHOULD NOT be done unless it is not possible to send it in headers, and an intermediary MUST NOT send Proxy-Status as a trailer field unless it has also sent a corresponding Proxy-Status header field in the same message, so that the trailer value's ordering relative to other intermediaries is preserved. |
'only' seems inappropriate here, since we subsequently advise that it needs to be sent as a header to be included in trailers.
There was a problem hiding this comment.
I accepted this change, but how am I misreading it? The sentence says:
[...], Proxy-Status can be only appended to the trailers of the outgoing message, since the headers have already been sent.
so we're referring to the case where headers have been already sent, forwarding of the response body has been interrupted, and the only way that we can send Proxy-Status is in the trailers. How is it inappropriate?
I'm not a native speaker, but every time I read it without "only", it just felt wrong.
There was a problem hiding this comment.
Ah, never mind, you're correct; I misread. I'll merge this and reinstate it.
Co-authored-by: Mark Nottingham <mnot@mnot.net>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Signed-off-by: Piotr Sikora piotrsikora@google.com