Skip to content
This repository has been archived by the owner on Jul 10, 2024. It is now read-only.

PROC-1471: Encode cookie for Rfc6265CookieProcessor #161

Merged
merged 5 commits into from
Apr 8, 2024

Conversation

zacharygoodwin
Copy link
Contributor

No description provided.

Copy link
Collaborator

@jamesallen-indeed jamesallen-indeed left a comment

Choose a reason for hiding this comment

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

Will this have any unintended consequences? For example, if some team is parsing the cookie and expecting a comma?

@zacharygoodwin
Copy link
Contributor Author

Will this have any unintended consequences? For example, if some team is parsing the cookie and expecting a comma?

That is a good question, I think it would. However, I think it is better practice and a improvement that they likely should handle as well. Thoughts? @stevenchen-indeed

vanhaxl
vanhaxl approved these changes Mar 21, 2024
@vanhaxl vanhaxl self-requested a review March 21, 2024 22:35
@vanhaxl
Copy link
Contributor

vanhaxl commented Mar 21, 2024

Will this have any unintended consequences? For example, if some team is parsing the cookie and expecting a comma?

That is a good question, I think it would. However, I think it is better practice and a improvement that they likely should handle as well. Thoughts? @stevenchen-indeed

I think we should consider this. It seems there are cases where this cookie value prForceGroups is being used source graph

@zacharygoodwin zacharygoodwin changed the title PROC-1643: Encode cookie for Rfc6265CookieProcessor PROC-1671: Encode cookie for Rfc6265CookieProcessor Mar 25, 2024
@zacharygoodwin zacharygoodwin changed the title PROC-1671: Encode cookie for Rfc6265CookieProcessor PROC-1471: Encode cookie for Rfc6265CookieProcessor Mar 25, 2024
@zacharygoodwin zacharygoodwin merged commit e3b0c8e into indeedeng:master Apr 8, 2024
1 check passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants