Skip to content
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

Investigate - Cookies are not thread safe when using CookieContainer #274

Closed
jiekexinduo opened this issue Mar 14, 2018 · 9 comments · Fixed by #281
Closed

Investigate - Cookies are not thread safe when using CookieContainer #274

jiekexinduo opened this issue Mar 14, 2018 · 9 comments · Fixed by #281
Labels
question Initially seen a question could become a new feature or bug or closed ;)

Comments

@jiekexinduo
Copy link

I have found a Bug about cookie in Ocelot ; let me describe it : I have two pc,both of them will request the ocelot which deployed in cloud server and both of them are in the same LAN ; then the website behind the ocelot will get the cookie2 from pc1's request; in fact the cookie2 belongs to pc2; how can i sove this problem?

@Anduin2017
Copy link

Could you please produce more information about the repro steps, I mean the process of operation to trigger this issue?

@jiekexinduo
Copy link
Author

PC1 request Server set-cookie "guid":xxxx0001; PC2 request Server set-cookie "guid":xxxx0002; then PC1request server and bring with "guid":xxxx0001 to server ;PC2 PC1request server and bring with "guid":xxxx0002 to server; i Capture the request bag and find that it's write ; But when i debug in websit behind the ocelot and find that pc1'request has "guid":xxxx0002

@TomPallister
Copy link
Member

@jiekexinduo thanks for your interest in the project! Can you paste your configuration.json here and I will try and replicate the problem!

@TomPallister
Copy link
Member

@Anduin2017 thanks for helping out :)

@TomPallister TomPallister added the question Initially seen a question could become a new feature or bug or closed ;) label Mar 14, 2018
@jiekexinduo
Copy link
Author

thanks for reply.I remove the ocelot and use the front web request my web api directly and it's right; so i doubt that maybe the ocelot do some otherthing caused this result. today i continue debug my code and i found a strange thing: when i use Request.Headers.GetCookies(“guid”) it had two guid key ,one of them belonged to PC2; and the other is my needs; and I used the HttpContext.Current.Request.Cookies["guid"].Value to get the cookie value; and this code always get the error guid when i request webapi through the ocelot;

@jiekexinduo
Copy link
Author

{

"ReRoutes": [
{
"DownstreamPathTemplate": "/sso/{everything}",
"DownstreamScheme": "http",
"DownstreamHostAndPorts": [
{
"Host": "localhost",
"Port": 8080
}
],
"UpstreamPathTemplate": "/sso/{everything}",
"UpstreamHttpMethod": [ "Post", "Get", "OPTIONS" ],
"ReRouteIsCaseSensitive": false
}
],
"GlobalConfiguration": {
"BaseUrl": "http://localhost:5000"
}
}

@jiekexinduo
Copy link
Author

I'm sorry ,I add a config of ocelot and solved this problem.UseCookieContainer:false ; Force the httpclient not to use cookie; if you have free time ,can you explain why the httpclient request downstream with setting "UseCookies false" can bring my upstream request's cookie to my webapi

@TomPallister
Copy link
Member

@jiekexinduo I think this is a bug I will look into it more. Ocelot caches HttpClient instances and the cookie container is also cached as part of this. When you make your request this is what causes the issue.

When you don’t use th cookie container it doesn’t matter that it caches!

@TomPallister
Copy link
Member

Will leave issue open until I can look at this to remind me!

@TomPallister TomPallister reopened this Mar 15, 2018
@TomPallister TomPallister changed the title cookie is formless from two pc Investigate - Cookies are not thread safe when using CookieContainer Mar 16, 2018
TomPallister pushed a commit that referenced this issue Mar 16, 2018
…probably going to be a redesign where we hold a reference to the cookie container and empty it if needed
TomPallister added a commit that referenced this issue Mar 17, 2018
TomPallister added a commit that referenced this issue Mar 17, 2018
TomPallister added a commit that referenced this issue Mar 17, 2018
…d docs, added tests to show cookie container behaviour and without container
TomPallister added a commit that referenced this issue Mar 17, 2018
TomPallister added a commit that referenced this issue Mar 17, 2018
* #274 added acceptance tests, need to work out failing unit tests but probably going to be a redesign where we hold a reference to the cookie container and empty it if needed

* #274 updated code coverage value

* #274 offloaded cache logic to builder in preparation for adding state

* #274 hacked something together but this is not right approach

* #274 updated defaults and docs

* #274 updated code coverage
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Initially seen a question could become a new feature or bug or closed ;)
Projects
None yet
3 participants