Skip to content

ALWAYS include the Vary Accept-Encoding header when response compression is enabled#55092

Merged
BrennanConroy merged 14 commits intodotnet:mainfrom
pedrobsaila:48008
Apr 10, 2026
Merged

ALWAYS include the Vary Accept-Encoding header when response compression is enabled#55092
BrennanConroy merged 14 commits intodotnet:mainfrom
pedrobsaila:48008

Conversation

@pedrobsaila
Copy link
Copy Markdown
Contributor

@pedrobsaila pedrobsaila commented Apr 11, 2024

ALWAYS include the Vary Accept-Encoding header when response compression is enabled

  • You've read the Contributor Guide and Code of Conduct.
  • You've included unit or integration tests for your change, where applicable.
  • You've included inline docs for your change, where applicable.
  • There's an open issue for the PR that you are making. If you'd like to propose a new feature or change, please open an issue to discuss the change or find an existing issue.

Summary of the changes (Less than 80 chars)

Description

ALWAYS include the Vary Accept-Encoding header when response compression is enabled

Fixes #48008

@ghost ghost added the area-middleware Includes: URL rewrite, redirect, response cache/compression, session, and other general middlewares label Apr 11, 2024
@dotnet-policy-service dotnet-policy-service Bot added the community-contribution Indicates that the PR has been added by a community member label Apr 11, 2024
@dotnet-policy-service dotnet-policy-service Bot added the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Apr 22, 2024
@pedrobsaila pedrobsaila reopened this May 14, 2024
@dotnet-policy-service dotnet-policy-service Bot added this to the 9.0-preview5 milestone May 14, 2024
@adityamandaleeka
Copy link
Copy Markdown
Member

adityamandaleeka commented May 14, 2024

@pedrobsaila Sorry for the longer-than-usual delay for getting things reviewed. This is still on our radar.

@pedrobsaila
Copy link
Copy Markdown
Contributor Author

pedrobsaila commented May 15, 2024

Sorry for the longer-than-usual delay for getting things reviewed. This is still on our radar.

no worries

@mkArtakMSFT mkArtakMSFT removed this from the 9.0-preview5 milestone Sep 9, 2024
BrennanConroy
BrennanConroy reviewed Mar 18, 2026
View reviewed changes
Comment thread src/Middleware/ResponseCompression/src/ResponseCompressionProvider.cs
}

/// <inheritdoc />
public bool CheckRequestAcceptsCompression(HttpContext context)
Copy link
Copy Markdown
Member

@BrennanConroy BrennanConroy Mar 18, 2026

Choose a reason for hiding this comment

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

While I like the simplicity of this change, it essentially makes CheckRequestAcceptsCompression useless, and the logs for requests without accept-encoding are a bit odd.

It might be better to change the ResponseCompressionMiddleware to handle including the vary header when not compressing the response.

Copy link
Copy Markdown
Contributor Author

@pedrobsaila pedrobsaila Mar 21, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

I see there is another reviewed PR trying to fix the same issue, should I continue working on this PR o close it ?

Copy link
Copy Markdown
Member

@BrennanConroy BrennanConroy Mar 21, 2026

Choose a reason for hiding this comment

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

I would prefer continuing with the fix on this PR if you're interested.

Copy link
Copy Markdown
Contributor Author

@pedrobsaila pedrobsaila Mar 22, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

I duplicated the code from ResponseCompressionBody that sets Vary header in ResponseCompressionMiddleware. Should I move that code to the provider inside a new method so I can call it from both the body and middleware without duplication ?

Copy link
Copy Markdown
Member

@BrennanConroy BrennanConroy Mar 24, 2026

Choose a reason for hiding this comment

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

Should I move that code to the provider inside a new method so I can call it from both the body and middleware without duplication ?

Yeah that would be good, maybe something like bool ShouldCompressResponseCommon(IResponseCompressionProvider, HttpConext)?

Copy link
Copy Markdown
Member

@BrennanConroy BrennanConroy Mar 30, 2026

Choose a reason for hiding this comment

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

Oh, sorry I didn't see you said move to the provider. We don't need any new public API surface for this, just make it a static private method and call it from both the middleware and ResponseCompressionBody

This was referenced Mar 22, 2026
Open
Open
This was referenced Mar 25, 2026
Open
Closed
Merged
Closed
Merged
Open
Merged
BrennanConroy
BrennanConroy approved these changes Apr 2, 2026
View reviewed changes
Comment thread src/Middleware/ResponseCompression/src/ResponseCompressionMiddleware.cs Outdated
Comment thread src/Middleware/ResponseCompression/src/ResponseCompressionMiddleware.cs Outdated
This was referenced Apr 3, 2026
Open
Open
This was referenced Apr 4, 2026
Closed
Open
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
@BrennanConroy BrennanConroy merged commit 3b9acb9 into dotnet:main Apr 10, 2026
24 checks passed
@BrennanConroy
Copy link
Copy Markdown
Member

BrennanConroy commented Apr 10, 2026

Thanks for sticking with this @pedrobsaila!

@dotnet-policy-service dotnet-policy-service Bot added this to the 11.0-preview4 milestone Apr 10, 2026
@dotnet-maestro dotnet-maestro Bot mentioned this pull request Apr 10, 2026
Merged
@pedrobsaila pedrobsaila deleted the 48008 branch April 10, 2026 03:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@BrennanConroy BrennanConroy BrennanConroy approved these changes

Assignees

@BrennanConroy BrennanConroy

Labels

area-middleware Includes: URL rewrite, redirect, response cache/compression, session, and other general middlewares community-contribution Indicates that the PR has been added by a community member pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun

Projects

None yet

Milestone

11.0-preview4

Development

Successfully merging this pull request may close these issues.

ALWAYS include the Vary: Accept-Encoding header when response compression is enabled

4 participants

@pedrobsaila @adityamandaleeka @BrennanConroy @mkArtakMSFT