Skip to content

[webview_flutter] Platform implementations for getCookies #11037#11386

Open
khaled-0 wants to merge 15 commits intoflutter:mainfrom
khaled-0:implementations
Open

[webview_flutter] Platform implementations for getCookies #11037#11386
khaled-0 wants to merge 15 commits intoflutter:mainfrom
khaled-0:implementations

Conversation

@khaled-0
Copy link
Copy Markdown
Contributor

List which issues are fixed by this PR. You must list at least one issue.
#10833 #11037

Pre-Review Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the gemini-code-assist bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.

Footnotes

  1. Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling. 2

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request implements the getCookies method for PlatformWebViewCookieManager in both the Android and WKWebView packages. The changes include updated Pigeon definitions, native implementations for cookie retrieval, and updates to the example application to utilize the new API. Feedback identifies a bug in Android cookie parsing when values contain equals signs, incorrect domain matching logic in the WKWebView implementation that violates RFC 6265, and a missing nullability annotation in the Android native code. Typographical errors in the changelogs were also noted.

Comment on lines +100 to +107
final List<String> cookieValue = cookie.split('=');
webViewCookies.add(
WebViewCookie(
name: cookieValue.first,
value: cookieValue.last,
domain: url.toString(),
),
);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

critical

The current cookie parsing logic is incorrect if a cookie's value contains an equals sign (=). Using cookie.split('=') and taking cookieValue.last will only capture the part of the value after the last =.

To correctly parse the cookie, you should take everything after the first = as the value.

Suggested change
final List<String> cookieValue = cookie.split('=');
webViewCookies.add(
WebViewCookie(
name: cookieValue.first,
value: cookieValue.last,
domain: url.toString(),
),
);
final List<String> cookieParts = cookie.split('=');
webViewCookies.add(
WebViewCookie(
name: cookieParts.first,
value: cookieParts.skip(1).join('='),
domain: url.toString(),
),
);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant