search

nruth / show_me_the_cookies

Cookie manipulation for Capybara (view, delete, …)
161 stars 44 forks source link

Escape cookie values when creating with Webkit driver #38

Closed darkphnx closed 6 years ago

darkphnx commented 8 years ago

Without this, non-cookie safe characters will be stored incorrectly. Using Rack::Utils.escape mirrors the behaviour use in the Rack::Test driver.

nruth commented 8 years ago

Hi! This sounds like a good thing to fix.

Could you add a test for it?

The files involved should be https://github.com/nruth/show_me_the_cookies/blob/master/spec/shared_examples_for_api.rb and https://github.com/nruth/show_me_the_cookies/blob/master/spec/app/set_cookie.rb (sinatra cookies docs)

darkphnx commented 8 years ago

Shouldn't be a problem. Looks like I'll need to unescape the cookies when reading them back too.

nruth commented 6 years ago

This isn't as straightforward as I'd thought.

My worry is we're changing what we're given, and may be masking a bug in the app or test script. Setting valid cookie strings is the responsibility of the app/javascript/test script.

Having skim-read https://stackoverflow.com/questions/1969232/allowed-characters-in-cookies and tried out our tested drivers to see what they do with a value like ab;cd I'm leaning towards removing any escaping/encoding (so from the rack-test adapter) and throwing an error if rack-utils escaping changes the string. Probably have an option to turn that error off, so people can fight with raw strings and idiosyncrasies of their particular driver if they want to.

I'll merge this then make some changes.