This repository has been archived by the owner. It is now read-only.

https.request response objects aren't readable streams #3882

Closed
lerouxb opened this Issue Aug 17, 2012 · 3 comments

Comments

Projects
None yet
3 participants

lerouxb commented Aug 17, 2012

UPDATE 2: I was completely confused here. The problem is really that https.get doesn't take a url string, I didn't notice the error (or it wasn't particularly clear or obvious) and then I tried to read from the request as if it was working/valid anyway.


I'm sending the response object I get from http.request() straight to graphicsmagick's identify and it works fine. If I try the same thing with an https url and therefore https.request() it fails badly. This is for resources that are accessible under the exact same urls securely (https, port 443 ) or over plain http (port 80), so I know they are exactly the same images and as long as the response objects behave as readable streams in both cases the problem cannot lie with graphicsmagick (https://github.com/aheckmann/gm).

The documentation shows that http.request (http://nodejs.org/api/http.html#http_http_request_options_callback) and https.request (http://nodejs.org/api/https.html#https_https_request_options_callback) have the same method signature, take the same options and both even have the same .get() shortcuts, so it would make sense if the response objects that get sent to the callback act as readable streams in both cases.

It looks like the intention is for them to have the same interface. Is this correct?

This is with node version 0.8.7.

UPDATE: Here's a gist that demonstrates the issue: https://gist.github.com/3382480

koichik added a commit to koichik/node that referenced this issue Aug 18, 2012

https: make https.get() accept a URL
https.get() now accepts either a URL (as a string) or an options object.
Refs #2859.
Fixes #3882.

koichik commented Aug 18, 2012

@lerouxb - Thanks for the report.
workaround:

    var req = get(require('url').parse(url), function(res) {

Can someone review 853718d?

@koichik A lot of people have asked about and sent in pull requests like 853718d, myself included. url.parse was modified so that it would match up with http.get and https.get.

lerouxb commented Aug 18, 2012

@kolchik Thanks! I can't believe I didn't try that especially since I already had the url parsed to see if I should route it to http or https...

@lerouxb lerouxb closed this Aug 18, 2012

koichik added a commit that referenced this issue Aug 24, 2012

https: make https.get() accept a URL
https.get() now accepts either a URL (as a string) or an options object.

Refs #2859.
Fixes #3882.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.