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

http response not emitting data #2457

Closed
legege opened this issue Jan 3, 2012 · 7 comments
Closed

http response not emitting data #2457

legege opened this issue Jan 3, 2012 · 7 comments
Labels

Comments

@legege
Copy link

legege commented Jan 3, 2012

I'm trying to proxy a MJPEG HTTP stream and I found an issue with the http module or the http parser. A MJPEG HTTP response doesn't have a Content-Length header, and should normally, read the response body until EOF. However, I found that in particular cases, the http module doesn't read until EOF, and simply close the connection after having received the headers.

Here is the code I'm using to proxy the request:

var http = require('http');
var sys = require('util');
var url = require('url');

var mjpegUrl = "http://admin:[email protected]/cgi/mjpg/mjpg.cgi";

var server = http.createServer(function(req, res) {
  var options = url.parse(mjpegUrl);

  var mjpegRequest = http.request(options, function(mjpegResponse) {
    console.log("...response received");
    res.writeHead(200, {
      'Content-Type': mjpegResponse.headers['content-type'],
      'Cache-Control': 'no-cache, no-store, must-revalidate',
      'Expires': 'Mon, 01 Jul 1980 00:00:00 GMT',
      'Pragma': 'no-cache'
    });

    console.log("...response");
    mjpegResponse.setEncoding(null);
    mjpegResponse.on('data', function(chunk) {
      //console.log("...data");
      res.write(chunk, 'binary');
    });
    mjpegResponse.on('end', function () {
      console.log("...end");
      res.end();
    });
    mjpegResponse.on('close', function () {
      console.log("...close");
    });
  });
  mjpegRequest.on('error', function(e) {
    console.log('problem with request: ' + e.message);
  });
  mjpegRequest.end();
});
server.listen(5080);

The problem occurs when proxying a MJPEG stream from a TRENDNet TV-IP422 camera. It works fine with an Axis camera.

I have wireshark traces of the issue. The only difference I see from a working response versus this one, is with the way TCP segments are sent back (on TV-IP422, the first segment is for the HTTP response status code, the next is for HTTP response headers, the next is for the MJPEG stream, etc; on Axis, the first segment is for HTTP response status code + headers + first part of MJPEG stream).

I can expose the camera on the Internet, please contact me.

@bnoordhuis
Copy link
Member

Can you post the wireshark traces? With what version of Node are you testing it?

@legege
Copy link
Author

legege commented Jan 3, 2012

I'm using node v0.6.6.

Here are 2 traces taken with curl:
http://tolan.legege.com/~legege/node/issue2457/notok-trendnet.pcap
http://tolan.legege.com/~legege/node/issue2457/ok-axis.pcap

Let me know if you need the traces taken with node (they are similar, but the notok trace will not have the body).

@bnoordhuis
Copy link
Member

Correct me if I'm wrong but it looks like the TRENDNet camera doesn't send a full response. It should be 5858 bytes big according to the Content-Length header but it seems only 3916 bytes were actually sent.

@legege
Copy link
Author

legege commented Jan 3, 2012

This is only partial because I hit "ctrl+c". The Content-Length header is not part of the HTTP headers, it's part of the MJPEG stream (we get a Content-Length header for each image, directly in the body, separated by the boundary ("myboundary")).

@legege
Copy link
Author

legege commented Jan 3, 2012

Here is another trace, with more packets:
http://tolan.legege.com/~legege/node/issue2457/notok-trendnet2.pcap

You'll see that the --myboundary with Content-Type/Content-Length is repeated in the body. These are headers for the MJPEG standard.

@koichik
Copy link

koichik commented Jan 4, 2012

@legege: Thanks for the report. This is similar to #1711.

reproduce:

var net = require('net');
var http = require('http');

var server = net.createServer({ allowHalfOpen: true }, function(socket) {
  console.log('SERVER: writing status line');
  socket.write('HTTP/1.1 200 ok\r\n');
  setTimeout(function() {
    console.log('SERVER: writhing headers');
    //socket.write('Connection: close\r\n');
    socket.write('Content-Type: text/plain\r\n\r\n');
    setTimeout(function() {
      console.log('SERVER: writing body');
      socket.write('Hello');
      setTimeout(function() {
        console.log('SERVER: closing');
        socket.end();
      }, 10);
    }, 10);
  }, 10);
}).listen(3000, function() {
  var req = http.get({ port: 3000 }, function(res) {
    res.on('data', function(buf) {
      console.log('CLIENT: data', buf.toString());
    });
    res.on('end', function() {
      console.log('CLIENT: end');
      server.close();
    });
  });
});

result:

$ node 2457.js
SERVER: writing status line
SERVER: writhing headers
CLIENT: end
SERVER: writing body
SERVER: closing

http.ClientResponse emitted 'end' event before the server sent the message body.
If uncomment socket.write('Connection: close\r\n'):

$ node 2457.js
SERVER: writing status line
SERVER: writhing headers
SERVER: writing body
CLIENT: data Hello
SERVER: closing
CLIENT: end

So, this problem is related with Keep-Alive.

In this case (neither Content-Length nor Transfer-Encoding), I think that http-parser should read the message body until connection is closed (RFC 2616, section 4.4-5). However, http-parser decides the message-length with 0.
I will send the patch to http-parser.

@legege: You should not use setEncoding(null). It does not mean to use a byte stream.

@legege
Copy link
Author

legege commented Jan 4, 2012

@koichik Thanks a lot for this quick investigation and patch!

ry pushed a commit to nodejs/http-parser that referenced this issue Jan 6, 2012
With HTTP/1.1, if neither Content-Length nor Transfer-Encoding is present,
section 4.4 of RFC 2616 suggests http-parser needs to read a response body
until the connection is closed (except the response must not include a body)

See also nodejs/node-v0.x-archive#2457.
Fixes #72
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

3 participants