Skip to content

Improve documentation for CookiePolicyMiddleware and CookieOptions#54968

Merged
captainsafia merged 3 commits intodotnet:mainfrom
bseptember:CookiePolicyMiddleware-modifies-CookieOptions-Fix-Doc
Apr 8, 2024
Merged

Improve documentation for CookiePolicyMiddleware and CookieOptions#54968
captainsafia merged 3 commits intodotnet:mainfrom
bseptember:CookiePolicyMiddleware-modifies-CookieOptions-Fix-Doc

Conversation

@bseptember
Copy link
Contributor

@bseptember bseptember commented Apr 5, 2024

Improve documentation for CookiePolicyMiddleware and CookieOptions

  • 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)

Added more detail in the doc. to warn developers of the recommended usage to avoid potential issues related to reusing CookieOptions instances.

Description

This pull request deals with Issue #53875 which is related to CookiePolicyMiddleware that's set up with UseCookiePolicy directly modifies the CookieOptions that's passed in to IResponseCookies.Append. This can lead to bugs where the modifications "bleed" over to unintended cookies if the CookieOptions is reused for multiple cookies, for example if it's stored in a static field in a controller.

Therefore, the documentation has been updated accordingly.

Fixes #53875 (documentation changed)

@ghost ghost added the area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions label Apr 5, 2024
@bseptember bseptember closed this Apr 5, 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 5, 2024
@dotnet-policy-service dotnet-policy-service bot added this to the 9.0-preview4 milestone Apr 5, 2024
@bseptember bseptember changed the title Improve documentation. for CookiePolicyMiddleware and CookieOptions Improve documentation for CookiePolicyMiddleware and CookieOptions Apr 5, 2024
@bseptember bseptember reopened this Apr 5, 2024
@bseptember
Copy link
Contributor Author

@dotnet-policy-service agree company="Retro Rabbit"

@amcasey amcasey added area-middleware Includes: URL rewrite, redirect, response cache/compression, session, and other general middlewares and removed area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions labels Apr 5, 2024
@captainsafia
Copy link
Contributor

cc: @Rick-Anderson @tdykstra for docs review.

Copy link
Contributor

@tdykstra tdykstra left a comment

Choose a reason for hiding this comment

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

Some suggestions for you to consider.

Copy link
Contributor

@Rick-Anderson Rick-Anderson left a comment

Choose a reason for hiding this comment

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

Approve with @tdykstra suggestions.

@captainsafia captainsafia merged commit 79ef5e3 into dotnet:main Apr 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CookiePolicyMiddleware modifies CookieOptions, leading to bugs if it's reused

6 participants