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.

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

H1SpecExceptions add option to allow LF without proceeding CR #1475

Merged
merged 5 commits into from
Apr 10, 2021

Conversation

Scottmitch
Copy link
Member

Motivation:
The HTTP/1.x spec allows for optional support to accept LF without
proceeding CR as a valid separator in some scenarios [1]. Some older
implementations rely upon this behavior and interoperability would be
improved by allowing opt-in support.

[1] https://tools.ietf.org/html/rfc7230#section-3.5
Although the line terminator for the start-line and header fields is
the sequence CRLF, a recipient MAY recognize a single LF as a line
terminator and ignore any preceding CR.

Modifications:

  • Update HttpObjectDecoder to support LF without CR

Result:
H1SpecExceptions allows for opt-in support for LF without proceeding
CR.

Motivation:
The HTTP/1.x spec allows for optional support to accept LF without
proceeding CR as a valid separator in some scenarios [1]. Some older
implementations rely upon this behavior and interoperability would be
improved by allowing opt-in support.

[1] https://tools.ietf.org/html/rfc7230#section-3.5
Although the line terminator for the start-line and header fields is
the sequence CRLF, a recipient MAY recognize a single LF as a line
terminator and ignore any preceding CR.

Modifications:
- Update HttpObjectDecoder to support LF without CR

Result:
H1SpecExceptions allows for opt-in support for  LF without proceeding
CR.
@Scottmitch
Copy link
Member Author

build failure attributed to #1224


/**
* Allows interpreting connection closures as the end of HTTP/1.1 messages if the receiver did not receive any
* part of the payload body before the connection closure.
*
* @deprecated Use {@link #prematureClosureBeforePayloadBody(boolean)}.
Copy link
Member Author

Choose a reason for hiding this comment

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

@idelpivnitskiy - this is inline with previous discussion we've had regarding builders allowing state to be controlled externally and re-usable.

Copy link
Member

@idelpivnitskiy idelpivnitskiy left a comment

Choose a reason for hiding this comment

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

Thank you!
LGTM after comments addressed

@Scottmitch Scottmitch merged commit cf51335 into apple:main Apr 10, 2021
@Scottmitch Scottmitch deleted the http_lf branch April 10, 2021 04:10
idelpivnitskiy added a commit to idelpivnitskiy/servicetalk that referenced this pull request Jun 21, 2021
… in apple#1475

Motivation:

`H1SpecExceptions.Builder#allowPrematureClosureBeforePayloadBody()`
option was deprecated in apple#1475 and can be removed now. Users can use
`allowPrematureClosureBeforePayloadBody(boolean)` overload as an
alternative.

Modifications:

- Remove `H1SpecExceptions.Builder#allowPrematureClosureBeforePayloadBody()`;

Result:

No deprecated
`H1SpecExceptions.Builder#allowPrematureClosureBeforePayloadBody()` API.
idelpivnitskiy added a commit that referenced this pull request Jun 22, 2021
… in #1475 (#1638)

Motivation:

`H1SpecExceptions.Builder#allowPrematureClosureBeforePayloadBody()`
option was deprecated in #1475 and can be removed now. Users can use
`allowPrematureClosureBeforePayloadBody(boolean)` overload as an
alternative.

Modifications:

- Remove `H1SpecExceptions.Builder#allowPrematureClosureBeforePayloadBody()`;

Result:

No deprecated
`H1SpecExceptions.Builder#allowPrematureClosureBeforePayloadBody()` API.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants