Skip to content

[release/7.0] Fix RequestBody logging when zero byte read is used #47811

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

Merged
merged 2 commits into from
May 2, 2023

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Apr 20, 2023

Backport of #47794 to release/7.0

/cc @BrennanConroy

Fix RequestBody logging when zero byte read is used

Description

In 7.0, zero-byte reads became the default in Kestrel, when using the PipeReader feature, which weren't handled by the HttpLoggingMiddleware. It would treat the zero-byte read as the end of what it should log for the request body, and since zero-byte reads are the first read, nothing would be logged.

Customer Impact

Using the HttpLoggingMiddleware may stop logging request bodies when upgrading from 6.0 to 7.0.

Customer reported issue #47216

Regression?

  • Yes
  • No

Regressed in 7.0

Risk

  • High
  • Medium
  • Low

Small change and well understood problem.

Verification

  • Manual (required)
  • Automated

Packaging changes reviewed?

  • Yes
  • No
  • N/A

@ghost ghost added the area-runtime label Apr 20, 2023
@ghost ghost added this to the 7.0.x milestone Apr 20, 2023
@ghost
Copy link

ghost commented Apr 20, 2023

Hi @github-actions[bot]. 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.

@BrennanConroy BrennanConroy added the Servicing-consider Shiproom approval is required for the issue label Apr 20, 2023
@ghost
Copy link

ghost commented Apr 20, 2023

Hi @github-actions[bot]. 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.

@BrennanConroy BrennanConroy added Servicing-approved Shiproom has approved the issue and removed Servicing-consider Shiproom approval is required for the issue labels Apr 25, 2023
@ghost
Copy link

ghost commented Apr 25, 2023

Hi @github-actions[bot]. This PR was just approved to be included in the upcoming servicing release. Somebody from the @dotnet/aspnet-build team will get it merged when the branches are open. Until then, please make sure all the CI checks pass and the PR is reviewed.

@amcasey
Copy link
Member

amcasey commented May 1, 2023

@BrennanConroy Good to go?

@BrennanConroy
Copy link
Member

No, you need to wait for servicing branches to open. Usually Will will handle merging then.

@wtgodbe wtgodbe merged commit 1ae68f1 into release/7.0 May 2, 2023
@wtgodbe wtgodbe deleted the backport/pr-47794-to-release/7.0 branch May 2, 2023 20:31
@ghost ghost modified the milestones: 7.0.x, 7.0.7 May 2, 2023
@ghost ghost added the area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework label May 2, 2023
@amcasey amcasey added area-middleware Includes: URL rewrite, redirect, response cache/compression, session, and other general middlewares 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-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework area-middleware Includes: URL rewrite, redirect, response cache/compression, session, and other general middlewares Servicing-approved Shiproom has approved the issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants