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

Broken querystring after qs.stringify #411

Closed
kof opened this issue Jul 7, 2014 · 8 comments
Closed

Broken querystring after qs.stringify #411

kof opened this issue Jul 7, 2014 · 8 comments

Comments

@kof
Copy link
Contributor

kof commented Jul 7, 2014

I have the following url

http://git.ozlabs.org/?p=yaboot.git;a=rss;f=second/file.c;opt=--no-merges

parsed output of it is

{ protocol: 'http:',
slashes: true,
auth: null,
host: 'git.ozlabs.org',
port: null,
hostname: 'git.ozlabs.org',
hash: null,
search: '?p=yaboot.git;a=rss;f=second/file.c;opt=--no-merges',
query: { p: 'yaboot.git;a=rss;f=second/file.c;opt=--no-merges' },
pathname: '/',
path: '/?p=yaboot.git;a=rss;f=second/file.c;opt=--no-merges',
href: 'http://git.ozlabs.org/?p=yaboot.git;a=rss;f=second/file.c;opt=--no-merges' }

@kof
Copy link
Contributor Author

kof commented Jul 7, 2014

If I change pathname to path, it breaks 3 tests.

@kof kof closed this as completed Jul 7, 2014
@kof kof reopened this Jul 7, 2014
@kof
Copy link
Contributor Author

kof commented Jul 7, 2014

  1. req.query(String) should work with url query-string:
  Uncaught AssertionError: expected { name: [ 'tobi', 'tobi' ], age: '2' } to equal { name: 'tobi', age: '2' }
  + expected - actual

   {
     "age": "2",
  +  "name": "tobi"
  -  "name": [
  -    "tobi",
  -    "tobi"
  -  ]
   }
  1. req.query(Object) should append to the original query-string:
  Uncaught AssertionError: expected { name: [ 'tobi', 'tobi' ], order: 'asc' } to equal { name: 'tobi', order: 'asc' }
  + expected - actual

   {
  +  "name": "tobi",
  -  "name": [
  -    "tobi",
  -    "tobi"
  -  ],
     "order": "asc"
   }
  1. req.query(Object) should retain the original query-string:
  Uncaught AssertionError: expected { name: [ 'tobi', 'tobi' ] } to equal { name: 'tobi' }
  + expected - actual

   {
  +  "name": "tobi"
  -  "name": [
  -    "tobi",
  -    "tobi"
  -  ]
   }

@kof
Copy link
Contributor Author

kof commented Jul 7, 2014

Oh thats complicated to track down. So at the end the path looks like

/?p=yaboot.git%3Ba%3Drss%3Bf%3Dsecond%2Ffile.c%3Bopt%3D--no-merges

@kof
Copy link
Contributor Author

kof commented Jul 7, 2014

Finally understood the problem.

qs.stringify encodes the url and in this case it breaks things, here:

try {
var querystring = qs.stringify(this.qs);
req.path += querystring.length
? (~req.path.indexOf('?') ? '&' : '?') + querystring
: '';
} catch (e) {
return this.callback(e);
}

@kof
Copy link
Contributor Author

kof commented Jul 7, 2014

No idea where to fix that correctly

@kof kof changed the title pathname vs. path Broken querystring after qs.stringify Jul 7, 2014
@ikokostya
Copy link

Before commit 629bf9b any string passed in Request#query just appends to querystring without encoding.

@defunctzombie
Copy link
Contributor

Is there an issue here? Is this about the client version or the node version?

@defunctzombie
Copy link
Contributor

Closing this for no response. If this is still broken (sounds likely) can we make a simple explanation of the issue and failing test. The current issue is hard to follow :)

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

No branches or pull requests

3 participants