Skip to content

Conversation

@Tratcher
Copy link
Member

@Tratcher Tratcher commented May 24, 2022

Handle spaces after request line

Handle malformed requests with a space between the request line and the first header

Description

A partner team reported compat issues with a client that is sending a space between the HTTP/1.1 request line and the first header. Kestrel (correctly) rejects this request as invalid. IIS allows this. The client will eventually be fixed, but they've requested a 6.0 patch to unblock them for the next few months. We do not plan to port this change to main/7.0.

Fixes #41824

Customer Impact

Preventing customer adoption of Kestrel due to client comapt issues.

Regression?

  • Yes
  • No

Risk

  • High
  • Medium
  • Low

This is an opt in adjustment to the parser.

Verification

  • Manual (required)
  • Automated

Packaging changes reviewed?

  • Yes
  • No
  • N/A

@Tratcher Tratcher added this to the 6.0.x milestone May 24, 2022
@Tratcher Tratcher self-assigned this May 24, 2022
@ghost ghost added the area-runtime label May 24, 2022
@ghost
Copy link

ghost commented May 24, 2022

Hi @Tratcher. If this is not a tell-mode PR, please make sure to follow the instructions laid out in the servicing process document.
Otherwise, please add tell-mode label.

@Tratcher Tratcher added the Servicing-consider Shiproom approval is required for the issue label May 24, 2022
@ghost
Copy link

ghost commented May 24, 2022

Hi @Tratcher. Please make sure you've updated the PR description to use the Shiproom Template. Also, make sure this PR is not marked as a draft and is ready-to-merge.

To learn more about how to prepare a servicing PR click here.

@Tratcher Tratcher marked this pull request as ready for review May 24, 2022 22:18
@Tratcher Tratcher force-pushed the tratcher/release/6.0/hspace branch from e1ee716 to 4f7d721 Compare May 25, 2022 18:30
if (_allowSpaceAfterRequestLine)
{
// Skip a space after the request line
if (reader.TryPeek(out byte s) && s == ByteSpace)
Copy link
Member

Choose a reason for hiding this comment

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

One space?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, from the reports we think that's all we need.

@Pilchie
Copy link
Member

Pilchie commented May 26, 2022

This was approved over email.

@Pilchie Pilchie added Servicing-approved Shiproom has approved the issue and removed Servicing-consider Shiproom approval is required for the issue labels May 26, 2022
@Pilchie Pilchie modified the milestones: 6.0.x, 6.0.7 May 26, 2022
@dougbu dougbu merged commit a7c039b into dotnet:release/6.0 Jun 7, 2022
@Tratcher Tratcher deleted the tratcher/release/6.0/hspace branch August 10, 2022 17:27
@amcasey amcasey added area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions and removed area-runtime labels Jun 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions Servicing-approved Shiproom has approved the issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants