-
-
Notifications
You must be signed in to change notification settings - Fork 0
chore: sync upstream PR #8249 - fix(ios): remove percent-encoding of cookie values in CapacitorCookies #35
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
base: plus
Are you sure you want to change the base?
Conversation
WalkthroughTwo public helper methods ( Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
cab4363 to
73ab898
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
ios/Capacitor/Capacitor/Plugins/CapacitorCookieManager.swift(0 hunks)ios/Capacitor/Capacitor/Plugins/CapacitorCookies.swift(1 hunks)
💤 Files with no reviewable changes (1)
- ios/Capacitor/Capacitor/Plugins/CapacitorCookieManager.swift
🧰 Additional context used
🧬 Code graph analysis (1)
ios/Capacitor/Capacitor/Plugins/CapacitorCookies.swift (1)
ios/Capacitor/Capacitor/Plugins/CapacitorCookieManager.swift (2)
setCookie(49-56)setCookie(58-64)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: test-android
- GitHub Check: test-core
| let expires = call.getString("expires", "") | ||
| let path = call.getString("path", "") | ||
| cookieManager!.setCookie(url, key, cookieManager!.encode(value), expires, path) | ||
| cookieManager!.setCookie(url, key, value, expires, path) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find CapacitorCookieManager to understand the setCookie implementation
fd -t f "CapacitorCookieManager" --type swiftRepository: Cap-go/capacitor-plus
Length of output: 237
🏁 Script executed:
#!/bin/bash
# Find CapacitorCookieManager using correct syntax
fd -e swift CapacitorCookieManagerRepository: Cap-go/capacitor-plus
Length of output: 125
🏁 Script executed:
#!/bin/bash
# Read the CapacitorCookieManager file to understand setCookie implementation
cat -n ios/Capacitor/Capacitor/Plugins/CapacitorCookieManager.swiftRepository: Cap-go/capacitor-plus
Length of output: 5505
🏁 Script executed:
#!/bin/bash
# Find test files for cookies
fd -e swift | xargs grep -l "setCookie\|CapacitorCookie" | grep -i testRepository: Cap-go/capacitor-plus
Length of output: 47
🏁 Script executed:
#!/bin/bash
# Read the CapacitorCookies.swift file to see the context around line 34
cat -n ios/Capacitor/Capacitor/Plugins/CapacitorCookies.swiftRepository: Cap-go/capacitor-plus
Length of output: 2809
🏁 Script executed:
#!/bin/bash
# Search for the encode method that was removed
rg -n "func encode" --type swiftRepository: Cap-go/capacitor-plus
Length of output: 5157
🏁 Script executed:
#!/bin/bash
# Check git log for changes to CapacitorCookieManager
cd ios/Capacitor/Capacitor/Plugins
git log --oneline -20 CapacitorCookieManager.swift 2>/dev/null | head -20Repository: Cap-go/capacitor-plus
Length of output: 121
🏁 Script executed:
#!/bin/bash
# Show the diff for the commit that removed encoding
git show 73ab8980 -- ios/Capacitor/Capacitor/Plugins/CapacitorCookies.swift | head -100Repository: Cap-go/capacitor-plus
Length of output: 2910
🏁 Script executed:
#!/bin/bash
# Show the diff for CapacitorCookieManager.swift to see encoding removal
git show 73ab8980 -- ios/Capacitor/Capacitor/Plugins/CapacitorCookieManager.swift | head -200Repository: Cap-go/capacitor-plus
Length of output: 5169
🏁 Script executed:
#!/bin/bash
# Check if there were previous Cookie files that might have been replaced
git log --all --name-status --oneline | grep -i cookie | head -20Repository: Cap-go/capacitor-plus
Length of output: 1430
🏁 Script executed:
#!/bin/bash
# Check the parent commit to see the previous state
git show 73ab8980^:ios/Capacitor/Capacitor/Plugins/CapacitorCookies.swift 2>/dev/null | head -50Repository: Cap-go/capacitor-plus
Length of output: 47
🏁 Script executed:
#!/bin/bash
# Get the full diff with more context to see if encode method was removed
git diff 73ab8980^..73ab8980 -- ios/Capacitor/Capacitor/Plugins/CapacitorCookieManager.swift | grep -A5 -B5 "encode\|value" | head -60Repository: Cap-go/capacitor-plus
Length of output: 106
🌐 Web query:
HTTPCookie.cookies(withResponseHeaderFields:for:) RFC 6265 special characters encoding iOS
💡 Result:
Short answer
- RFC 6265 restricts cookie-name and cookie-value to US‑ASCII characters; several characters (space, DQUOTE, comma, semicolon, backslash and CTLs) are not allowed in an unquoted cookie value. User agents parse Set‑Cookie using the RFC algorithm (which is slightly more permissive about trimming WSP). (rfc-editor.org)
- Foundation’s HTTPCookie/HTTPCookieStorage follows that parsing behavior when building HTTPCookie objects from response headers; different platform implementations (macOS vs Linux, older iOS versions) have had small differences in which header-keys are considered or how non‑conforming cookies are handled. In practice you must encode characters that are forbidden by RFC 6265 before sending them in Set‑Cookie. (forums.swift.org)
What this means for your iOS code
- If your server needs to store arbitrary data (non‑ASCII or characters like space, ", ,, ;, ), encode the value on the server (recommended: percent-encode using an allowed ASCII set or use base64) and decode in the client. RFC 6265 allows user agents to accept quoted values but forbids those special characters unescaped; relying on quoting is fragile across agents. (rfc-editor.org)
Practical recommendations
- Server: percent‑encode or base64 the cookie value (percent is interoperable; base64 avoids reserved characters but may include “=” which is allowed in cookie values). (rfc-editor.org)
- Client (iOS): read cookies via HTTPCookie.cookies(withResponseHeaderFields:for:) or HTTPCookieStorage; decode the stored value after retrieving the HTTPCookie.value. Be aware of platform differences (inspect header keys if you see missing cookies). (forums.swift.org)
References
- RFC 6265 — HTTP State Management Mechanism (cookie grammar and parsing algorithm). (rfc-editor.org)
- Discussion / interop notes and implementation differences (Swift Forums). (forums.swift.org)
- iOS/macOS behavior reports and examples (Stack Overflow). (stackoverflow.com)
If you want, tell me the exact characters or an example cookie value you need to send and I’ll show the exact percent‑encoding or base64 approach and the decoding code on iOS.
Citations:
- 1: https://www.rfc-editor.org/rfc/rfc6265.html?utm_source=openai
- 2: https://forums.swift.org/t/set-cookie-headers-are-handled-differently-on-macos-and-linux/68147?utm_source=openai
- 3: https://www.rfc-editor.org/rfc/rfc6265.html?utm_source=openai
- 4: https://www.rfc-editor.org/rfc/rfc6265.html?utm_source=openai
- 5: https://forums.swift.org/t/set-cookie-headers-are-handled-differently-on-macos-and-linux/68147?utm_source=openai
- 6: https://www.rfc-editor.org/rfc/rfc6265.html?utm_source=openai
- 7: https://forums.swift.org/t/set-cookie-headers-are-handled-differently-on-macos-and-linux/68147?utm_source=openai
- 8: https://stackoverflow.com/questions/48148804/httpcookie-cookieswithresponseheaderfields-headerfields-for-url-returns-0-i?utm_source=openai
🌐 Web query:
Capacitor PR #8249 cookies percent encoding fix
💡 Result:
I couldn’t find a GitHub pull request ionic-team#8249 for Capacitor. I searched the ionic-team/capacitor repo and related docs — there are recent cookie-related issues (e.g. “CapacitorCookies percent-encodes cookies on iOS (and not on Android)” as issue ionic-team#8248) and general cookie reports, but no PR numbered 8249. Sources:
- Capacitor issue list showing cookie-related issues (includes [Bug]: CapacitorCookies percent-encodes cookies on iOS (and not on Android) ionic-team/capacitor#8248). [1]
- Capacitor Cookies plugin docs (behavior and config). [2]
- Example GitHub cookie issues (3rd-party/persistent cookie reports). [3][4]
If you want, I can:
- watch the repository and notify you if PR fix(ios): remove percent-encoding of cookie values in CapacitorCookies ionic-team/capacitor#8249 appears, or
- search for the PR in forks/other repos or provide more detail on the cookie bugs and workarounds. Which would you prefer?
References
- GitHub issues list — ionic-team/capacitor. [https://github.com/ionic-team/capacitor/issues]
- Capacitor Cookies plugin docs. [https://capacitorjs.com/docs/apis/cookies]
- Issue: 3rd party cookies not working on capacitor iOS (bug: 3rd party cookies not working on capacitor iOS ionic-team/capacitor#6302). [https://github.com/bug: 3rd party cookies not working on capacitor iOS ionic-team/capacitor#6302]
- Issue: Persistent Cookie Storage Issue in iOS App (bug: Persistent Cookie Storage Issue in iOS App ionic-team/capacitor#6809). [https://github.com/bug: Persistent Cookie Storage Issue in iOS App ionic-team/capacitor#6809]
🌐 Web query:
Capacitor issue #8248 percent-encode cookies iOS Android
💡 Result:
There’s an open GitHub issue for this: "CapacitorCookies percent-encodes cookies on iOS (and not on Android)" (issue ionic-team#8251) — opened Nov 21, 2025 on the ionic-team/capacitor repo. Related cookie problems (third‑party/persistence) are also tracked in other Capacitor issues (e.g. ionic-team#6302). [1][2]
Sources:
- ionic-team/capacitor issues list (shows issue [Bug]: iOS Prerequisite Checks & Cordova Package Generation Race ionic-team/capacitor#8251 title and bug: 3rd party cookies not working on capacitor iOS ionic-team/capacitor#6302). [1]
- Capacitor issue bug: 3rd party cookies not working on capacitor iOS ionic-team/capacitor#6302 (3rd party cookies on iOS). [2]
🏁 Script executed:
#!/bin/bash
# Search for test files in the repository
find . -name "*test*" -o -name "*spec*" | grep -i cookieRepository: Cap-go/capacitor-plus
Length of output: 47
HTTPCookie does not automatically encode/escape special characters—validation or encoding must be implemented before interpolating user input into the Set-Cookie header.
RFC 6265 forbids unencoded special characters (semicolons, equals signs, spaces, quotes, commas, backslashes, control characters) in cookie values. Per the iOS Foundation documentation, HTTPCookie.cookies(withResponseHeaderFields:for:) follows RFC 6265 parsing and will either reject or mispars non-compliant headers—it does not perform encoding.
The current code at line 60 of CapacitorCookieManager.swift directly interpolates the user-provided value into the Set-Cookie header string without encoding or validation:
let field = ["Set-Cookie": "\(key)=\(value); expires=\(expires ?? ""); path=\(path ?? "/")"]Required actions:
- Either re-introduce encoding (percent-encoding or base64) before interpolation, or add explicit validation to reject non-RFC-6265-compliant values
- Add test cases covering edge cases (values containing
;,=, spaces, quotes, etc.) - Ensure consistency with Android implementation—verify whether Android silently fails, encodes, or validates on the server side
- Document the expected behavior and any breaking changes for users who previously relied on automatic encoding
🤖 Prompt for AI Agents
In ios/Capacitor/Capacitor/Plugins/CapacitorCookies.swift around line 34, the
code directly interpolates user-provided cookie values into the Set-Cookie
header which can include RFC-6265-disallowed characters; update the
implementation to either percent-encode or base64-encode the cookie value (or
perform strict validation and reject non-compliant values) before constructing
the "Set-Cookie" header, add unit tests that cover values containing ; = space "
, backslash and control characters, verify and align behavior with the Android
implementation (encode, reject, or preserve) and update the plugin docs/release
notes to describe the chosen behavior and any breaking changes for existing
users.
Upstream PR Sync
This PR syncs changes from an external contributor's PR on the official Capacitor repository.
Original PR
Automation
Synced from upstream by Capacitor+ Bot
Summary by CodeRabbit
Refactor
Breaking Changes
✏️ Tip: You can customize this high-level summary in your review settings.