Skip to content
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

fix #331 cookies parsing #351

Open
wants to merge 188 commits into
base: master
Choose a base branch
from
Open

fix #331 cookies parsing #351

wants to merge 188 commits into from

Conversation

cTatu
Copy link

@cTatu cTatu commented Mar 13, 2019

remove first and last extra quotes as suggested in #331

lejard-h and others added 30 commits November 6, 2017 10:50
use standard Activity as calling class to support FlutterFragmentActivity
add eval javascript
sink onUrlChanged ...
android: add rect, fullScreen, userAgent, eval
Android: crash in close a hidden webview
[Android] Back button navigates back now instead of closing webview
Add pub badge
charafau and others added 4 commits March 13, 2019 21:35
hotfix widget back to initialChild after webview is tapped on Android
remove first and last extra quotes
@mikuhl-dev
Copy link

I think this will still cause the spaces in the cookie names.

@cTatu
Copy link
Author

cTatu commented Mar 14, 2019

Well, I can't think of how. The tokens used for splitting are special characters like ; and = so spaces in the key or values names will be ok

@cTatu cTatu changed the title fix #331 splitting of cookies fix #331 cookies parsing Jul 13, 2019
@charafau
Copy link
Collaborator

@cTatu Thank you for PR. I'd like to merge this one, but due to my time limitation, could you please provide easy to test / steps to reproduce, so I can check if it's working?

Thanks

@cTatu
Copy link
Author

cTatu commented Jul 15, 2019

@charafau Sure, I will check with all sorts of values and upload a test file. Think that will be enough?

@charafau
Copy link
Collaborator

@cTatu anything will do really :)

@cTatu
Copy link
Author

cTatu commented Aug 23, 2019

Ok here you have the test comparing the old and new methods ;)

Also fixes #418 taking the first = as separator

@cremor
Copy link

cremor commented Aug 23, 2019

I assume this also fixes #525 since you removed the hardcoded access at index 1?

edit: Or maybe not. If split[1] failed, skip(1) might fail too.

@cTatu
Copy link
Author

cTatu commented Aug 23, 2019

Yes, also fixes #525, the case where the cookiesString contains only \"\" is handled gracefully by

.where((c) => c.isNotEmpty) so stops there and doesn't reach the forEach loop.

I have updated the dartpad test with an empty string case if you want to check it out!

@cremor
Copy link

cremor commented Aug 23, 2019

Good to hear, thanks.

But I can't get your DartPad test to run. I always get

Uncaught exception:
FormatException: Illegal RegExp pattern (SyntaxError: invalid regexp group)
(?<=").+?(?=")

@cTatu
Copy link
Author

cTatu commented Aug 23, 2019

That's weird, it even works on my phone's browser 😕

@mikuhl-dev
Copy link

mikuhl-dev commented Aug 23, 2019 via email

@cremor
Copy link

cremor commented Aug 23, 2019

Ah, yes, that's it. https://caniuse.com/#feat=js-regexp-lookbehind

@shinriyo
Copy link

when does it be released?

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.