Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

regression: https.get(url, ...) returns partial content #25749

Closed
rbelouin opened this issue Jul 22, 2015 · 6 comments
Closed

regression: https.get(url, ...) returns partial content #25749

rbelouin opened this issue Jul 22, 2015 · 6 comments

Comments

@rbelouin
Copy link

Hi,

It seems that the https module is broken for Node.js 0.12.7, it may also be the case for other 0.12.x versions of Node, but I haven't built them yet.

I am getting some content which is available via HTTP or HTTPS, and using the http and the https modules does not give me the same result, while Firefox, wget and Node.js 0.10.39 do.

Here is the piece of code reproducing the bug:

var http = require("http");
var https = require("https");

var httpString;
var httpsString;

var onend = function() {
  if(httpString && httpsString) {
    console.log("httpsString equals to httpString: " + (httpsString == httpString));
    console.log("httpsString is smaller than httpString: " + (httpsString.length < httpString.length));
    console.log("httpsString is the beginning of httpString: " + (httpString.indexOf(httpsString) == 0));
  }
};

http.get("http://ccapi-preprod.cleverapps.io/v2/application.wadl", function(res) {
  var data = "";

  res.on("data", function(chunk) {
    data += chunk;
  });

  res.on("end", function() {
    httpString = data;
    onend();
  });
});

https.get("https://ccapi-preprod.cleverapps.io/v2/application.wadl", function(res) {
  var data = "";

  res.on("data", function(chunk) {
    data += chunk;
  });

  res.on("end", function() {
    httpsString = data;
    onend();
  });
});

We expect the following output (and this is what we get with Node.js 0.10.39 and 0.10.40):

httpsString equals to httpString: true
httpsString is smaller than httpString: false
httpsString is the beginning of httpString: true

And this is what we actually get with Node.js 0.12.7:

httpsString equals to httpString: false
httpsString is smaller than httpString: true
httpsString is the beginning of httpString: true

Technical information:
OS: Linux mufasa 4.0.7-2-ARCH #1 SMP PREEMPT Tue Jun 30 07:50:21 UTC 2015 x86_64 GNU/Linux
Node.js: 0.12.7, installed with nvm (as for Node.js 0.10.39)

@papandreou
Copy link

The way you concatenate and stringify the response chunks won't work if a chunk ends with a partial character. Try this instead, maybe that helps:

  var chunks = [];

  res.on("data", function(chunk) {
    chunks.push(chunk);
  });

  res.on("end", function() {
    httpString = Buffer.concat(chunks).toString('utf-8');
    onend();
  });

... and equivalently for httpsString.

Edit: Going res.setEncoding('utf-8'); would also do the trick.

@rbelouin
Copy link
Author

Thanks for the tip ! But it is still failing.

I updated the test:

var http = require("http");
var https = require("https");

var httpString;
var httpsString;

var onend = function() {
  if(httpString && httpsString) {
    var assertion = httpsString == httpString;

    console.log(assertion ? "test passed" : "test failed");
    process.exit(assertion ? 0 : 1);
  }
};

http.get("http://ccapi-preprod.cleverapps.io/v2/application.wadl", function(res) {
  var chunks = [];

  res.on("data", function(chunk) {
    chunks.push(chunk);
  });

  res.on("end", function() {
    httpString = Buffer.concat(chunks).toString("utf-8");
    onend();
  });
});

https.get("https://ccapi-preprod.cleverapps.io/v2/application.wadl", function(res) {
  var chunks = [];

  res.on("data", function(chunk) {
    chunks.push(chunk);
  });

  res.on("end", function() {
    httpsString = Buffer.concat(chunks).toString("utf-8");
    onend();
  });
});

@rbelouin
Copy link
Author

After running a git bisect, 15a5a4a seems to be the first bad commit.

rbelouin added a commit to rbelouin/node that referenced this issue Jul 27, 2015
Some https requests returns truncated content, although everything works
when requesting the same resource with http. This regression appears
since 15a5a4a, and making the https globalAgent keep the connection
alive fix it.

Closes nodejs#25749
rbelouin added a commit to rbelouin/node that referenced this issue Jul 27, 2015
Some https requests returns truncated content, although everything works
when requesting the same resource with http. This regression appears
since 15a5a4a, and making the https globalAgent keep the connection
alive fix it.

Closes nodejs#25749
divarvel added a commit to divarvel/wadl-client that referenced this issue Jul 29, 2015
This is a workaround for nodejs/node-v0.x-archive#25749
If keepAlive is not explicitely enabled, some requests are truncated.
@divarvel
Copy link

divarvel commented Sep 3, 2015

Any news on this issue?

@jasnell
Copy link
Member

jasnell commented Sep 3, 2015

@divarvel ... it hasn't been forgotten. It's actually on my rather long "short list" of items to look at further, just haven't had the opportunity to dig in on it.

@divarvel
Copy link

divarvel commented Sep 3, 2015

Ok, thanks for the update
On 3 Sep 2015 18:03, "James M Snell" notifications@github.com wrote:

@divarvel https://github.com/divarvel ... it hasn't been forgotten.
It's actually on my rather long "short list" of items to look at further,
just haven't had the opportunity to dig in on it.


Reply to this email directly or view it on GitHub
#25749 (comment)
.

divarvel added a commit to CleverCloud/clever-tools that referenced this issue Sep 23, 2015
Enable keepAlive on https requests in nodeJS

This is a workaround for nodejs/node-v0.x-archive#25749
If keepAlive is not explicitely enabled, some requests are truncated.
@Trott Trott closed this as completed Apr 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants