cloudhead / node-static

rfc 2616 compliant HTTP static-file server module, with built-in caching.
MIT License
2.17k stars 245 forks source link

`response.writeHead` method is called twice #208

Open anseki opened 6 years ago

anseki commented 6 years ago

Hi, thank you for the great module.

So, response.writeHead method is called twice when the request is not a GET request.

For example:

Save a content file to public/index.html:

<body>Hello, World!</body>

Run this code as a server:

'use strict';

const fileServer = new (require('node-static')).Server('./public');

require('http').createServer((request, response) => {
  request.addListener('end', () => {

    response.setHeader('foo', 'bar');
    fileServer.serve(request, response);

  }).resume();
}).listen(8080);

And run this code as a client (using request module):

'use strict';

require('request')({
  url: 'http://localhost:8080/index.html',
  method: 'HEAD'
}, (error, response, body) => {
  console.log('error: ', error);
  console.log('statusCode: ', response && response.statusCode);
  console.log('headers:');
  console.log(response && response.headers);
  console.log('body: ', body);
});

Then, an error "Can't set headers after they are sent." is thrown. This issue is relevant to https://github.com/cloudhead/node-static/issues/207.

BTW, when the response.setHeader method was not called, the error is not thrown even if the response.writeHead method is called twice. This is another issue about Node.js.

In Server.prototype.respondNoGzip, the response.writeHead method is called when requested file is not cached. https://github.com/cloudhead/node-static/blob/e994d29d620d0dc67cb1e23da764dbddbdd14059/lib/node-static.js#L333 And, in Server.prototype.finish, the response.writeHead method is called again when the request is not a GET request. https://github.com/cloudhead/node-static/blob/e994d29d620d0dc67cb1e23da764dbddbdd14059/lib/node-static.js#L125

Also, a response body is made (i.e. it is piped to response) even if the request is a HEAD request. (And that body may be discarded by the server.) https://github.com/cloudhead/node-static/blob/e994d29d620d0dc67cb1e23da764dbddbdd14059/lib/node-static.js#L379

I couldn't understand this comment: https://github.com/cloudhead/node-static/blob/e994d29d620d0dc67cb1e23da764dbddbdd14059/lib/node-static.js#L122 What is a case that the response.writeHead method must be called here? Anyway, it should check whether headers were already sent or not by using response.headersSent or response._header.

            if (!res._header) {
              res.writeHead(status, headers);
anseki commented 6 years ago

BTW, it seems that these are duplicated. https://github.com/cloudhead/node-static/blob/e994d29d620d0dc67cb1e23da764dbddbdd14059/lib/node-static.js#L303 https://github.com/cloudhead/node-static/blob/e994d29d620d0dc67cb1e23da764dbddbdd14059/lib/node-static.js#L311