Skip to content

Conversation

@riderx
Copy link
Member

@riderx riderx commented Nov 28, 2025

Upstream PR Sync

This PR syncs changes from an external contributor's PR on the official Capacitor repository.

Original PR

  • PR: #8249
  • Title: fix(ios): remove percent-encoding of cookie values in CapacitorCookies
  • Author: @michaelwolz

Automation

  • CI will run automatically
  • Claude Code will review for security/breaking changes
  • If approved, this PR will be auto-merged

Synced from upstream by Capacitor+ Bot

Summary by CodeRabbit

  • Refactor

    • Simplified cookie value handling in iOS by removing intermediate encoding/decoding steps. Cookie values are now processed directly without additional transformation layers.
  • Breaking Changes

    • Removed public helper methods for cookie value encoding and decoding from the iOS plugin API.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Nov 28, 2025

Walkthrough

Two public helper methods (encode and decode) were removed from CapacitorCookieManager, and the setCookie method in CapacitorCookies was updated to pass raw values instead of encoded ones, simplifying the cookie-setting flow.

Changes

Cohort / File(s) Summary
Encoding/Decoding Utilities Removed
ios/Capacitor/Capacitor/Plugins/CapacitorCookieManager.swift
Removed public encode(_ value: String) -> String and decode(_ value: String) -> String methods that performed percent-encoding and percent-decoding operations
Cookie Setting Logic Updated
ios/Capacitor/Capacitor/Plugins/CapacitorCookies.swift
Updated setCookie method to pass raw value parameter to cookie manager instead of calling encode(value), removing the explicit encoding step

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Verify that the removed encode and decode methods are not called elsewhere in the codebase
  • Confirm that removing explicit encoding does not break cookie handling or compatibility
  • Ensure the raw value assignment in setCookie is the intended behavior

Poem

🐰 No more encode, no more decode, cookies now travel their natural road!
Simplified paths through the iOS grove, where raw values flow and truly move.
Less code to carry, fewer helpers to weigh—simplicity wins the day! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title clearly describes the main change: removing percent-encoding of cookie values in CapacitorCookies, which matches the actual modifications (removing encode/decode methods and their usage).
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch sync/upstream-pr-8249

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@riderx riderx force-pushed the sync/upstream-pr-8249 branch from cab4363 to 73ab898 Compare November 28, 2025 15:01
Copy link

@coderabbitai coderabbitai bot left a 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

📥 Commits

Reviewing files that changed from the base of the PR and between 270d700 and 73ab898.

📒 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)
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find CapacitorCookieManager to understand the setCookie implementation
fd -t f "CapacitorCookieManager" --type swift

Repository: Cap-go/capacitor-plus

Length of output: 237


🏁 Script executed:

#!/bin/bash
# Find CapacitorCookieManager using correct syntax
fd -e swift CapacitorCookieManager

Repository: 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.swift

Repository: 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 test

Repository: 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.swift

Repository: 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 swift

Repository: 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 -20

Repository: 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 -100

Repository: 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 -200

Repository: 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 -20

Repository: 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 -50

Repository: 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 -60

Repository: 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:


🌐 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:

If you want, I can:

References

  1. GitHub issues list — ionic-team/capacitor. [https://github.com/ionic-team/capacitor/issues]
  2. Capacitor Cookies plugin docs. [https://capacitorjs.com/docs/apis/cookies]
  3. 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]
  4. 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:


🏁 Script executed:

#!/bin/bash
# Search for test files in the repository
find . -name "*test*" -o -name "*spec*" | grep -i cookie

Repository: 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:

  1. Either re-introduce encoding (percent-encoding or base64) before interpolation, or add explicit validation to reject non-RFC-6265-compliant values
  2. Add test cases covering edge cases (values containing ;, =, spaces, quotes, etc.)
  3. Ensure consistency with Android implementation—verify whether Android silently fails, encodes, or validates on the server side
  4. 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.

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.

3 participants