Skip to content

Allow OWIN hosts and applications to register default ICookieManager instances #486

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 1 commit into from
Dec 16, 2022

Conversation

kevinchalet
Copy link
Contributor

Related: #485.

/cc @Tratcher

@kevinchalet kevinchalet force-pushed the default_cookie_manager branch 2 times, most recently from 962bd1e to 4061b1b Compare December 15, 2022 11:41

if (manager is null)
{
app.Properties.Remove("infrastructure.CookieManager");
Copy link
Member

Choose a reason for hiding this comment

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

Use constants for the key names.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. I also updated the title to better reflect that these extensions can also be used by applications (and not just hosts) to override the default cookie managers.

@Tratcher Tratcher self-assigned this Dec 15, 2022
@kevinchalet kevinchalet force-pushed the default_cookie_manager branch from 20aadab to 1d84906 Compare December 15, 2022 21:28
@kevinchalet kevinchalet changed the title Allow OWIN servers to register default ICookieManager instances Allow OWIN hosts and applications to register default ICookieManager instances Dec 15, 2022
@kevinchalet kevinchalet force-pushed the default_cookie_manager branch from 1d84906 to e863f3a Compare December 15, 2022 21:48
@kevinchalet
Copy link
Contributor Author

Looks like the Microsoft.Owin.Host.HttpListener.Tests.OwinHttpListenerTests.PathAndPathBase_CorrectlySeperated test is flaky as it failed multiple times. Rebasing and force-pushing to trigger a new CI build 😄

@Tratcher Tratcher merged commit 423f80e into aspnet:main Dec 16, 2022
@Tratcher
Copy link
Member

Thanks

@Tratcher Tratcher added this to the 4.2.3 milestone Dec 16, 2022
@Tratcher
Copy link
Member

Tratcher commented Dec 16, 2022

I probably won't ship this right away, more likely in January. Try the nightly builds?

@kevinchalet
Copy link
Contributor Author

I will 😃 (I tested with the sandbox app when preparing this PR and the SystemWeb cookie manager was correctly resolved)

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