Regression 1.3.0 -> 1.4.0: "SYNTAX_ERR: This header is not allowed" on blocking request #33

Open
dfellis opened this Issue Jun 2, 2012 · 4 comments

2 participants

@dfellis

Hi Dan,

It's been a long time, so you probably don't remember me. (I contributed the original blocking code.)

There's unfortunately been a regression in version 1.4.0 that didn't exist in 1.3.0 when I try to do a blocking request. (On the bright side, it's forced me to finally open source my JSON-RPC client and server code.)

My JSON-RPC client is setting only two HTTP headers: Content-Length and Content-Type. I can't believe that Content-Type is what's being banned, so I assume setting Content-Length is no longer allowed?

Is 1.4.0 setting this value, now? Even if so, I think it'd be nicer if the API was closer to the browser and just silently dropped on setting this header and then set it itself (so the same code can work on browsers that do this, and browsers that don't set that header at all and therefore cause problems for servers that expect to see it, and why I added that line in in the first place).

I haven't looked into what's different between 1.3.0 and 1.4.0, just giving you a heads up. I can re-fork and fix if you'd like.

David

@driverdan
Owner

Modifying Content-Length is forbidden but is auto set if any data is included in the request so it shouldn't be an issue. Content-Type can be set so I'm not sure what the problem would be. Here's a list of headers that are currently forbidden from being modified per the spec:

  var forbiddenRequestHeaders = [
    "accept-charset",
    "accept-encoding",
    "access-control-request-headers",
    "access-control-request-method",
    "connection",
    "content-length",
    "content-transfer-encoding",
    "cookie",
    "cookie2",
    "date",
    "expect",
    "host",
    "keep-alive",
    "origin",
    "referer",
    "te",
    "trailer",
    "transfer-encoding",
    "upgrade",
    "user-agent",
    "via"
  ];

Someone else suggested relaxing the restrictions since browser security isn't an issue here. I'm planning on removing at least a few for the next release.

I'm surprised you're not seeing a security error in the browser when you try to set Content-Length.

@dfellis

I do it for backwards compatibility with older versions of Firefox, IE, and Opera. Some of them don't set that header at all if you don't specify the Content-Length. The others fire warnings, not errors, if you set it, and ignore the value. That's the only reason why my code sets Content-Length manually. Was hoping I wouldn't have to detect being in Node.js or not after the initial loading of the XMLHttpRequest lib.

@driverdan
Owner

So 1.4.0 isn't working? The only case I can see an issue is if you do a POST with empty data and are communicating with an older server that doesn't follow the specs. node-XHR currently doesn't set Content-Length for POST if there is no data. See issue #29 for details.

@dfellis

I ran wireshark to debug the issue before. The node-XHR code was killing the process before any request started because the Content-Length header was being set. The API usage was valid and works in browsers and earlier versions of node-XHR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment