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

http.get method() ignores query params when passing an URL as first argument #22162

Closed
gabriel-araujjo opened this issue Aug 6, 2018 · 7 comments
Labels
doc Issues and PRs related to the documentations. http Issues or PRs related to the http subsystem. question Issues that look for answers. url Issues and PRs related to the legacy built-in url module. whatwg-url Issues and PRs related to the WHATWG URL implementation.

Comments

@gabriel-araujjo
Copy link

gabriel-araujjo commented Aug 6, 2018

  • Version: v8.10.0
  • Platform: Linux 4.15.0-29-generic Ubuntu SMP Tue Jul 17 15:39:52 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux
  • Subsystem: http

When passing an URL with query params as the first argument to http.get() method, the query is ignored.

const URL = require('url');
const http = require('http');
const assert = require('assert');

function parseJsonResp(next) {
   return function(resp) {
     var raw = '';
     resp.on('data', function(chunk) {
       raw += chunk;
     });
     resp.on('end', function() {
       next(null, JSON.parse(raw));
     });
  }
};

const url = URL.parse('http://httpbin.org/get');
url.query = {foo: 'bar'}

http.get(url, parseJsonResp(function(err, obj) {
  assert(obj.args.foo === 'bar'); // It fails here
}));
@gabriel-araujjo gabriel-araujjo changed the title HTTP get ignores query params when passing address through url http.get method ignores query params when passing an URL as first argument Aug 6, 2018
@gabriel-araujjo gabriel-araujjo changed the title http.get method ignores query params when passing an URL as first argument http.get method() ignores query params when passing an URL as first argument Aug 6, 2018
@devsnek
Copy link
Member

devsnek commented Aug 6, 2018

http.get doesn't expect a whatwg url object, it expects an options argument similar to the output of url.parse()

@devsnek devsnek added question Issues that look for answers. http Issues or PRs related to the http subsystem. url Issues and PRs related to the legacy built-in url module. whatwg-url Issues and PRs related to the WHATWG URL implementation. labels Aug 6, 2018
@gabriel-araujjo
Copy link
Author

The documentation says that the first argument can be an Object, a string or an URL.

@rubys
Copy link
Member

rubys commented Aug 6, 2018

It is a very subtle thing... the documentation says URL, and what you passed was a Url. Click on the links to see the difference.

Try the following:

const http = require('http');
const assert = require('assert');

function parseJsonResp(next) {
   return function(resp) {
     var raw = '';
     resp.on('data', function(chunk) {
       raw += chunk;
     });
     resp.on('end', function() {
       next(null, JSON.parse(raw));
     });
  }
};

const url = new URL('http://httpbin.org/get');
url.searchParams.set('foo', 'bar')

http.get(url, parseJsonResp(function(err, obj) {
  assert(obj.args.foo === 'bar'); // It fails here
}));

Key changes:

  • Remove require('url')
  • Change URL.parse to new URL
  • Change url.query = to url.searchParams.set()

@gabriel-araujjo
Copy link
Author

gabriel-araujjo commented Aug 7, 2018

OK. Thanks! 👍. Shouldn't this be explicit on documentation?

@rubys
Copy link
Member

rubys commented Aug 7, 2018

@gabriel-araujjo just to be clear, this issue can be closed now? Ah, missed your edit. Suggestions on how to make it more clear?

@gabriel-araujjo
Copy link
Author

I guess an example with that distinction is enough.

@jasnell jasnell added the doc Issues and PRs related to the documentations. label Aug 12, 2018
rexagod added a commit to rexagod/node that referenced this issue Jun 6, 2020
@jasnell
Copy link
Member

jasnell commented Jun 25, 2020

The http URL has since been updated to support URL objects and properly handles the query string now so this can be closed.

@jasnell jasnell closed this as completed Jun 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. http Issues or PRs related to the http subsystem. question Issues that look for answers. url Issues and PRs related to the legacy built-in url module. whatwg-url Issues and PRs related to the WHATWG URL implementation.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants