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

Partial #186 Regression #358

Closed
andercs opened this issue Jun 1, 2021 · 1 comment
Closed

Partial #186 Regression #358

andercs opened this issue Jun 1, 2021 · 1 comment

Comments

@andercs
Copy link

andercs commented Jun 1, 2021

The fix created as of #186 appears to have since shifted to CookieSessionProvider.CreateSession in session_cookie.go.

The downside to this is that unless CreateSession is called, then net.SplitHostPort is never called. which can be somewhat problematic for testing purposes wherein an extraneous utility creates a fake token and we'd like to use CookieSessionProvider.DeleteSession to create the set-cookie header to delete it in our integration tests.

The suggested fix is to update the DefaultSessionProvider in new.go to set the Domain to opts.URL.Hostname, instead of opts.URL.Host. This silently strips the port, if any exists, from the given URL.

With that in mind, we might still want to leave the separate fix in CreateSession in case someone creates a CookieSessionProvider manually rather than by using New and getting a Middleware with a pre-populated CookieSessionProvider.

@andercs
Copy link
Author

andercs commented Jun 2, 2021

Actually put the cart before the horse and didn't look at the main branch. Looks like the issue is effectively resolved via #335. Apologies for making the issue.

Hope a new release gets cut soon, so that we can roll it out on our project! 😃

@andercs andercs closed this as completed Jun 2, 2021
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

No branches or pull requests

1 participant