Skip to content

Fix Multiple Cookie Issue: Properly Handle Multiple Set-Cookie Headers in ZitiUrlProtocol#283

Merged
scareything merged 1 commit intoopenziti:mainfrom
Dicky019:fix/header-set-cookie
Jul 25, 2025
Merged

Fix Multiple Cookie Issue: Properly Handle Multiple Set-Cookie Headers in ZitiUrlProtocol#283
scareything merged 1 commit intoopenziti:mainfrom
Dicky019:fix/header-set-cookie

Conversation

@Dicky019
Copy link
Copy Markdown
Contributor

This PR addresses an issue where multiple Set-Cookie headers in HTTP responses were not handled correctly by ZitiUrlProtocol. Previously, only one cookie would be processed, causing potential loss of additional cookies set by the server.

Key improvements:

  • Correctly parses and collects all Set-Cookie headers from HTTP responses.
  • Merges all cookie values into a single entry in the resulting dictionary, ensuring no cookies are missed.
  • Resolves issues with authentication, session management, or any functionality relying on multiple cookies.

This fix improves compatibility with servers that set multiple cookies and ensures more reliable client behavior.
Please review and let me know if further changes are needed. Thank you!

Copy link
Copy Markdown
Member

@scareything scareything left a comment

Choose a reason for hiding this comment

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

Hi and thanks for the PR! It looks good to me. Can you please sign a contributor license agreement if you haven't already?

@Dicky019
Copy link
Copy Markdown
Contributor Author

Hi and thanks for the PR! It looks good to me. Can you please sign a contributor license agreement if you haven't already?

Hi! Thanks a lot for checking it out 😊
Sure, I’ll go ahead and sign the CLA — will send it over shortly!

Let me know if there’s anything else I should do 🙌

@Dicky019
Copy link
Copy Markdown
Contributor Author

Dicky019 commented Jul 14, 2025

Hi and thanks for the PR! It looks good to me. Can you please sign a contributor license agreement if you haven't already?

Hey, thanks for the reminder!
I’ve just sent the signed Individual Contributor License Agreement over to  cla@netfoundry.io

Let me know if there’s anything else you need!

@scareything
Copy link
Copy Markdown
Member

Thanks for the cla. We have it. I just noticed that your commit on this PR is not signed, and we require signed commits. Could you please set up commit signing described here and amend your existing commit, then force-push?

@Dicky019
Copy link
Copy Markdown
Contributor Author

Thanks for the cla. We have it. I just noticed that your commit on this PR is not signed, and we require signed commits. Could you please set up commit signing described here and amend your existing commit, then force-push?

Hi there, thanks for the heads‑up! I’ll configure commit signing as described, amend the existing commit with a GPG signature, and force‑push the branch shortly. Let me know if you need anything else!

@Dicky019 Dicky019 force-pushed the fix/header-set-cookie branch 4 times, most recently from d47366d to ee50cd5 Compare July 24, 2025 13:35
@scareything
Copy link
Copy Markdown
Member

Hi. I see that the commit is now signed, Thanks!

I'm guessing the changes in deps/ on your most recent push were inadvertent, and they are breaking the build. Could you remove your changes to those non-swift files? I'm guessing a git reset would be the easiest way.

Thanks.

…o handle multiple Set-Cookie headers. Collect all Set-Cookie values into a single entry in the resulting dictionary.
@Dicky019 Dicky019 force-pushed the fix/header-set-cookie branch from ee50cd5 to 269aead Compare July 24, 2025 13:56
@Dicky019
Copy link
Copy Markdown
Contributor Author

Hi. I see that the commit is now signed, Thanks!

I'm guessing the changes in deps/ on your most recent push were inadvertent, and they are breaking the build. Could you remove your changes to those non-swift files? I'm guessing a git reset would be the easiest way.

Thanks.

Hi @scareything, sorry about that! I accidentally included changes to deps/ – I’ve reset those files and force‑pushed the branch. Build should be back to green now. Please let me know if you spot anything else. Thanks!

@scareything scareything merged commit 425f6d5 into openziti:main Jul 25, 2025
7 checks passed
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