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

duplicate http headers should not be dropped #2750

Closed
joachimkainz opened this issue Feb 14, 2012 · 8 comments
Closed

duplicate http headers should not be dropped #2750

joachimkainz opened this issue Feb 14, 2012 · 8 comments
Labels
Milestone

Comments

@joachimkainz
Copy link

http headers can very legitimately contain duplicate header variables. For instance, when working with WebDAV duplicate headers are used frequently. Here is an example for a header used by subversion:

OPTIONS /svn HTTP/1.1
User-Agent: SVN/1.7.2 neon/0.29.6
Keep-Alive: 
Connection: TE, Keep-Alive
TE: trailers
Host: jolira-tools.googlecode.com
Content-Type: text/xml
Accept-Encoding: gzip
DAV: http://subversion.tigris.org/xmlns/dav/svn/depth
DAV: http://subversion.tigris.org/xmlns/dav/svn/mergeinfo
DAV: http://subversion.tigris.org/xmlns/dav/svn/log-revprops
Content-Length: 104
Accept-Encoding: gzip

<?xml version="1.0" encoding="utf-8"?><D:options xmlns:D="DAV:"><D:activity-collection-set/></D:options>HTTP/1.1 200 OK

Other languages (such as Java) use an array to store headers for that reason.

The code change to support an array in lib/http.js is rather simple. I prototyped it in jolira@0face42.

@joachimkainz
Copy link
Author

Here is the change for the master branch: jolira@9392bee

@bnoordhuis
Copy link
Member

I hate going all standards lawyer on people but RFC 2616, section 4.2 has this to say about multiple headers:

Multiple message-header fields with the same field-name MAY be present in a message if and only if the entire field-value for that header field is defined as a comma-separated list [i.e., #(values)]. It MUST be possible to combine the multiple header fields into one "field-name: field-value" pair, without changing the semantics of the message, by appending each subsequent field-value to the first, each separated by a comma. The order in which header fields with the same field-name are received is therefore significant to the interpretation of the combined field value, and thus a proxy MUST NOT change the order of these field values when a message is forwarded.

Node is doing the right (or at least the standards approved) thing here by concatenating headers that are allowed to exist multiple times, like Accept-Encoding, and dropping duplicates of other headers, like the DAV header in your example.

For the sake of usability we could change that behaviour to, for example, concatenate "unknown" headers like we do for X-prefixed headers. Thoughts anyone?

@koichik
Copy link

koichik commented Feb 14, 2012

I would like to store ALL the multiple headers (even if they are invalid) as an array. But it may be related to #1448.
With v0.6, I am +1 to concatenate "unknown" headers.

@joachimkainz
Copy link
Author

You may want to take a look at this section of RFC 4918: http://tools.ietf.org/html/rfc4918#section-10.1

   This general-header appearing in the response indicates that the
   resource supports the DAV schema and protocol as specified.  All DAV-
   compliant resources MUST return the DAV header with compliance-class
   "1" on all OPTIONS responses.  In cases where WebDAV is only
   supported in part of the server namespace, an OPTIONS request to non-
   WebDAV resources (including "/") SHOULD NOT advertise WebDAV support.

   The value is a comma-separated list of all compliance class
   identifiers that the resource supports.  Class identifiers may be
   Coded-URLs or tokens (as defined by [RFC2616]).  Identifiers can
   appear in any order.  Identifiers that are standardized through the
   IETF RFC process are tokens, but other identifiers SHOULD be Coded-
   URLs to encourage uniqueness.

It seems like comma-delimiting values of duplicate headers it would work for the WebDAV.

@joachimkainz
Copy link
Author

The 2D-header array proposed in #1448 would address this issue.

@joachimkainz
Copy link
Author

I can work around this issue successfully by requiring the this file when starting my server: https://raw.github.com/jolira/node-subversion-hub/master/lib/http-patch.js

@cbiffle
Copy link

cbiffle commented Aug 3, 2012

This issue just bit me.

Node can't possibly know what headers "may" permit multiple values, because (1) I can define my own and (2) as of RFC6648 there's no easy way for you to know I've just made up a header (like looking for "X-").

@isaacs
Copy link

isaacs commented Aug 27, 2013

@cbiffle X-* headers are allowed to have multiples.

Is this still relevant now that incoming messages have a rawHeaders array of the raw keys and values?

var http = require('http');
var net = require('net');
var req =
  'OPTIONS /svn HTTP/1.1\r\n'+
  'User-Agent: SVN/1.7.2 neon/0.29.6\r\n'+
  'Keep-Alive: \r\n'+
  'Connection: TE, Keep-Alive\r\n'+
  'TE: trailers\r\n'+
  'Host: jolira-tools.googlecode.com\r\n'+
  'Content-Type: text/xml\r\n'+
  'Accept-Encoding: gzip\r\n'+
  'DAV: http://subversion.tigris.org/xmlns/dav/svn/depth\r\n'+
  'DAV: http://subversion.tigris.org/xmlns/dav/svn/mergeinfo\r\n'+
  'DAV: http://subversion.tigris.org/xmlns/dav/svn/log-revprops\r\n'+
  'Content-Length: 104\r\n'+
  'Accept-Encoding: gzip\r\n'+
  '\r\n'+
  '<?xml version="1.0" encoding="utf-8"?><D:options xmlns:D="DAV:"><D:activity-collection-set/></D:options>HTTP/1.1 200 OK\r\n';

var server = http.createServer(function(req, res) {
  console.error(req.rawHeaders);
  console.error(req.headers);
  res.setHeader('connection', 'close');
  res.setHeader('content-length', '2');
  res.end('ok');
  server.close();
});

server.listen(1337, function() {
  var client = net.connect(1337, function() {
    client.pipe(process.stdout);
  });
  client.end(req);
});

/*
[ 'User-Agent',
  'SVN/1.7.2 neon/0.29.6',
  'Keep-Alive',
  '',
  'Connection',
  'TE, Keep-Alive',
  'TE',
  'trailers',
  'Host',
  'jolira-tools.googlecode.com',
  'Content-Type',
  'text/xml',
  'Accept-Encoding',
  'gzip',
  'DAV',
  'http://subversion.tigris.org/xmlns/dav/svn/depth',
  'DAV',
  'http://subversion.tigris.org/xmlns/dav/svn/mergeinfo',
  'DAV',
  'http://subversion.tigris.org/xmlns/dav/svn/log-revprops',
  'Content-Length',
  '104',
  'Accept-Encoding',
  'gzip' ]
{ 'user-agent': 'SVN/1.7.2 neon/0.29.6',
  'keep-alive': '',
  connection: 'TE, Keep-Alive',
  te: 'trailers',
  host: 'jolira-tools.googlecode.com',
  'content-type': 'text/xml',
  'accept-encoding': 'gzip, gzip',
  dav: 'http://subversion.tigris.org/xmlns/dav/svn/depth',
  'content-length': '104' }
HTTP/1.1 200 OK
connection: close
content-length: 2
Date: Tue, 27 Aug 2013 18:21:11 GMT

ok
*/

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

5 participants