Loosing data when downloading via https with connection:close header

[matthiasg]

I am experiencing an issue that might be related to #8299. I have been hunting this down all morning and got the following so far:

When downloading a file via https i seem to miss a few bytes at the very end in a large number of cases when 'Connection':'Close' header is present in the Client Request.

e.g. This always works:

out = fs.createWriteStream('out.dat')
https.get('https://url').pipe(out)
out.once('finish',verifySizeOfDownload); // always works

This fails often:

  out = fs.createWriteStream('out.dat')

  var options = require('url').parse('https://myurl.com/test.dat');
  options.headers = {
    Accept: '*/*',
    Connection: 'close'   // when leaving this line out it works all the time. equivalent to passing a string instead of an options object.
  };

  https.get(options, function(res){
    res.pipe(out);
  })

 out.once('finish',verifySizeOfDownload); // often fails

The Server is a minimal 'express' server sending a static file back on a get request using res.sendfile.

PS:
it didnt help a lot of course that streams have the events 'close','end','finish' and some streams throw different e.g 'end' events ... but not others. But reading the source code and the docs, i would assume binding to finish it is correct to assume the file to be fully flushed. When counting the bytes as they are passed to the filestream using a transform stream, i also get a consistent _flush too early for the failing cases. (Also see #8209)

Because of that i looked higher in the stream stack and since a standard call always worked,
but adding a header 'Connection':'close' does not in a large number of cases (Windows and SmartOS). I would rather assume something is wrong with how the http/s requests handle the close and flush down stream.

Thoughts ?

[chrisdickinson]

Interesting bug! Could I ask what version of Node and what version of operating system you're seeing this on?

My suspicion is that it has to do with differences with how the underlying connection is working -- https is largely reusing the http.Agent logic, but handing it a different connection factory (tls.CryptoStream vs. net.Socket), so it looks like it's taking the same path up until it calls socket.destroySoon.

[matthiasg]

I am seeing that in SmartOS, Windows 8.1 and (not verified in the same way yet) MacOS. all up to date stable OS versions with most current nodejs (0.10.32)

We have had an issue which was caused by this behavior for months and I had to get that tracked down now. That means though the error was seemingly in there for at least the last 5-6 months.

[matthiasg]

it there anything i can do to help progress with this issue ? I am pretty sure this kind of error will sneak up on other people in production and its difficult to track

[misterdjules]

@matthiasg I haven't been able to reproduce the issue you describe. So far I tried on SmartOS and MacOS X, without success.

Could you please provide us with a fully working repro (including the URL of a file for which the issue happens at least sometimes)? Also, could you please tell us how often this issue happens?

[matthiasg]

We have worked around this issue for now by not using the workaround as described above. But if somebody wants to investigate then I will try to setup a server next week to reproduce from. It was quite consistent when it happened so it shouldn't be to difficult .

[argon]

@matthiasg I discovered #8299 and I want to try to come up with a fix but a reproducible test case is difficult. Would you still be able to supply me with a reproducible test case for this problem so I can check whether it's the same underlying problem?

[matthiasg]

Unsure .. But I can test again ... Which version spoiled I test against (node / iojs)

-----Original Message-----
From: "Andrew Naylor" notifications@github.com
Sent: ‎13.‎06.‎2015 17:13
To: "joyent/node" node@noreply.github.com
Cc: "matthiasg" mgt576@gmail.com
Subject: Re: [node] Loosing data when downloading via https withconnection:close header (#8488)

@matthiasg I discovered #8299 and I want to try to come up with a fix but a reproducible test case is difficult. Would you still be able to supply me with a reproducible test case for this problem so I can check whether it's the same underlying problem?
—
Reply to this email directly or view it on GitHub.

[misterdjules]

Possible duplicate of nodejs/node#3055.

[jasnell]

Closing as a duplicate of nodejs/node#3055