Skip to content

Set 'expires' to null for session cookies #354

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

Closed
wants to merge 1 commit into from

Conversation

madoni
Copy link

@madoni madoni commented Dec 31, 2015

Hi,
I noticed that the generated har contains invalid timestamp values for session cookies. Something like:
"expires" : "292269009-11-24T08:44:55.386-05:00",.
This results in exceptions while parsing the har file.
I realize this fix might be too simplistic... am open to suggestions!

Thanks!

@jekh
Copy link
Collaborator

jekh commented Jan 2, 2016

Thanks @madoni. I looked into this and it turns out, in addition to the issue you noticed, the cookie.maxAge() field was not being treated properly. The netty docs say it is in seconds, but we were treating it like milliseconds. So I created PR #355 which fixes the issue you spotted and the ms mismatch. Could you give that branch a try and see if it fixes your issue? If so I'll merge it in.

@madoni
Copy link
Author

madoni commented Jan 2, 2016

Hey @jekh, yes I think that should sort it out.
As long as the expires value for session cookies is null, it shouldn't appear in the corresponding har entry.

@jekh
Copy link
Collaborator

jekh commented Jan 2, 2016

Great. I'll close this PR now that the other one has been merged. Thanks for discovering this, @madoni!

@jekh jekh closed this Jan 2, 2016
@madoni
Copy link
Author

madoni commented Jan 2, 2016

Awesome! Glad I could contribute... My first attempt!

On Fri, Jan 1, 2016, 10:07 PM Jason Hoetger notifications@github.com
wrote:

Closed #354 #354.


Reply to this email directly or view it on GitHub
#354 (comment).

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