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

set cookie domain when clearing request tracker cookie #319

Closed
wants to merge 1 commit into from

Conversation

crewjam
Copy link
Owner

@crewjam crewjam commented Dec 14, 2020

No description provided.

@crewjam
Copy link
Owner Author

crewjam commented Dec 14, 2020

@tchap can you please take a look as see if this addresses the issue you referred to in your comment on PR #311 ?

@@ -54,6 +54,7 @@ func (t CookieRequestTracker) StopTrackingRequest(w http.ResponseWriter, r *http
return err
}
cookie.Value = ""
cookie.Domain = t.ServiceProvider.AcsURL.Host
Copy link

Choose a reason for hiding this comment

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

Shouldn't this be .Hostname() rather?

@tchap
Copy link

tchap commented Dec 14, 2020

I am going to test this now. I implemented my own request tracker and it seems to behave differently than the default one, so I have to find out what the difference is.

@tchap
Copy link

tchap commented Dec 14, 2020

@crewjam I am actually not affected by this, I implemented slightly changed CookieSessionProvider, which is now fixed in 61ad7bf , so I am waiting for that to be released. Cannot test this really...

@crewjam crewjam closed this Dec 14, 2020
@crewjam crewjam deleted the branch master December 14, 2020 16:55
@crewjam
Copy link
Owner Author

crewjam commented Dec 14, 2020

This was closed because of the master -> main transition. Repoened as #321

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