Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

wrote a test for weird characters in query string #214

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
3 participants

goblin commented Oct 4, 2012

When stub_request contains weird characters like + and / in the query string,
the error messages returned by rspec ("You can stub this request with")
contain "%20"s in the URL instead of %2B or %2F. This tests for that.

The test fails. I don't yet know how to fix the code to make it pass

wrote a test for weird characters in query string
When stub_request contains weird characters like + and / in the query string,
the error messages returned by rspec ("You can stub this request with")
contain "%20"s in the URL instead of %2B or %2F. This tests for that.
Owner

bblimke commented Oct 4, 2012

Thank you for this spec. I'll investigate this as soon as I can.

Owner

bblimke commented Feb 17, 2013

I'm sorry for the delay with this pull request. I'm still not sure what's the best way to handle + in url provided to webmock. as '+' or as space.

Collaborator

myronmarston commented Feb 18, 2013

FWIW, I've seen a fair bit of confusion (and, on occasion, been personally confused myself) about + in URLs. The TL;DR is that I think + should be treated as a space in the query string but not in the path. See this sinatra discussion for more.

Owner

bblimke commented Feb 28, 2013

+ in query params is not treated by WebMock as space anymore and is now encoded to %2B.

Thank you!

@bblimke bblimke closed this Feb 28, 2013

Owner

bblimke commented Mar 4, 2013

ok, maybe I was too fast with that change. @myronmarston so from what you wrote, when I see + in query params, it should be treated as a space right? Does that mean the test provided by @goblin is invalid?

@bblimke bblimke reopened this Mar 4, 2013

Collaborator

myronmarston commented Mar 4, 2013

ok, maybe I was too fast with that change. @myronmarston so from what you wrote, when I see + in query params, it should be treated as a space right?

When the query params are parsed, the standard is to treat them as being www-form-urlencoded -- which means a query string like ?q=a+b&c=5 would parse to { "q" => "a b", "c" => "5" }.

As I interpret it, + should only be treated as a space when parsing the query string as params. When considering the entire URI (as @goblin's test does), I don't think it should be treated in any particular way.

Owner

bblimke commented Mar 4, 2013

@myronmarston in @goblin's test, + appears only in the query string, not in the path, so the test assertions seem invalid.

Collaborator

myronmarston commented Mar 4, 2013

I agree the test looks invalid to me. My point was that his test is only looking at the entire URI so it shouldn't be treating any part in any particular way.

goblin commented Mar 4, 2013

So my test is only checking the output message from webmock in the case when it tells the user how to stub it.

When I had my query with a +, it failed a test, so I took the string given to me by "You should stub the request with", pasted it into my test, and it was still failing. I had to manually change the %20 into %2B to get the test to pass (all that if I remember correctly... it was a while ago)

goblin commented Mar 4, 2013

Perhaps it should just tell to stub with a + rather than %2B... dunno

@bblimke bblimke closed this Mar 5, 2013

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