Make private and custom headers case insensitive

[danvirsen]

I ran into a problem when testing two clients who use custom headers. The headers are only passed on from client A to client B, but not from client B to client A, even though the headers were defined in proxyRequestHeaders and proxyResponseHeaders. The issue is that client A defines the header in all lower case, and client B use mixed case.

I can't find anything that says that only standard headers should be case insensitive so I simplified the handling. Should some headers be case sensitive and if so, why?

I left the customHeaderNames array alone since I don't understand the purpose of it.

[davehorton]

People (and some systems) seem to be rather picky about how custom headers appear, case-wise, even though I agree the spec says they are case-insensitive so I am hesitant to make such a change as I am sure it will break some implementations out there. I would like to see logs of our scenario, though, to understand why the headers were not proxied. Perhaps you can attach logs with drachtio server loglevel debug?

[danvirsen]

That's unfortunate, but I understand the reasoning.

The log really doesn't say anything. If the P- or X- header doesn't match the case in proxyRequestHeaders it's simply not in the message from the NodeJs app to Drachtio.

Another issue that I forgot to mention is that the current implementation makes it difficult to check for these headers, since has and get for P- and X- headers are case sensitive as well. Ok, not difficult, but right now I to loop over the headers to make the keys lowercase before I make my checks.

Another approach might be to try and make only the search case-insensitive, but then the case in proxyRequestHeaders will be used, and not the case that was used by the client that made the request. So that might cause the same issues I suppose.

Something like this in lib/sip-parser/message.js:

getHeaderNameIfExists(hdr) {
  const hdrLowerCase = hdr.toLowerCase();
  return Object.keys(this.headers).find(h => h.toLowerCase() === hdrLowerCase);
}

get(hdr) {
  if (this.has(hdr)) {
    return this.headers[this.getHeaderNameIfExists(hdr)];
  }
}

has(hdr) {
  return !!this.getHeaderNameIfExists(hdr);
}

getParsedHeader(hdr) {
  const v = this.get(hdr);

  if (!v) {
    throw new Error('header not available');
  }

  const fn = parser.getParser(hdr.toLowerCase()) ;
  return fn({s:v, i:0}) ;
}

[danvirsen]

I slapped together a quick POC here: https://github.com/drachtio/drachtio-srf/compare/main...danvirsen:drachtio-srf:header-read-case-insensitive

This should keep the casing from the original message when copying over the headers, but the reads for all headers are case insensitive which solves the problem I ran into.

It's not pretty since there are now two functions in different places for fetching a header name. Plus I loop through all headers twice for each "get", so if you do a "has" check before the "get", all headers are looped through three times. But I think it shows the concept. Do you think this would avoid any issues with picky people/systems if cleaned up?

[danvirsen]

Closed in favour of https://github.com/drachtio/drachtio-srf/pull/165