Set Content-Length header on DELETE, PUT when no entity body

[katanacrimson]

Similar to request/request#41 - I'm seeing issues with an nginx server handling an API where DELETE operations to it are now returning HTTP 411 - there's a transfer-encoding header being injected somewhere along the way (not by superagent, likely nodejs itself because there's no content-length header) and it is causing severe problems with http requests.

[tj]

hmm sounds odd to me but I guess we could

[katanacrimson]

I'd appreciate that. I've had to monkey-patch it in on my own for the time being, and setting content-length to 0 for DELETE operations has managed to fix the problem. The transfer-encoding=chunked just ends up messing quite a bit up.

[defunctzombie]

@damianb Can you provide a simple example to show the problem please.

[jdesboeufs]

Any DELETE request through nginx without body :(
Hopefully it's easy to fix with req.set('content-length', 0).

curl seems to do this by default.

[soyuka]

@defunctzombie DELETE requires a content-length, at least with nginx (seems I had no issues with apache). If I try to curl DELETE it will send a 411 Length Required status.
Solution here is to set a Content-length header:

    if(this.method == 'DELETE') {
      this.set('Content-length', 0)
    }

atm I have a hack like this one to override the Request.prototype.request method.

[defunctzombie]

@soyuka can you make a PR with appropriate tests for this?

[soyuka]

@defunctzombie are there specific instructions to run tests? I did make test or make test-node but a lot is failing. Thanks.

[defunctzombie]

@soyuka make test-node should would. For browser tests you will need to setup zuul for cloud testing. Just steps 1 & 2 should be enough.

[soyuka]

@defunctzombie sorry made a lot of mistakes in my first commit it's late here ^^.

[kornelski]

As mentioned in #236 the HTTP RFC doesn't require DELETE request to have a Content-Length, so I think nginx is being too picky here.

Has anybody reported the issue to nginx? If the bug can be fixed in nginx I'd rather avoid adding a workaround superagent.

[katanacrimson]

a client workaround will still likely be necessary due to propagation of that patch/update across the web. even if nginx patches it today, it wouldn't land anytime soon in rhel, deb, ubuntu, so handling nginx's edge case would be necessary in the meantime.

[soyuka]

@damianb I see no client workaround here. How can you set the Content-Length header from the client (aka web browser)? See in the list of forbidden header name.

@pornel agreed, but it then say in OPTIONS that a request with an empty body MUST provide a Content-Length of 0. I'll search on the nginx side if there are mention of this particular issue.

[soyuka]

Continuing the discusison from the #711 PR here:

OPTIONS:

A server MUST generate a Content-Length field with a value of "0" if no payload body is to be sent in the response.

DELETE:

A payload within a DELETE request message has no defined semantics;
sending a payload body on a DELETE request might cause some existing
implementations to reject the request.

---

Yes indeed, I read both, but the OPTIONS paragraph was a more logical explanation to why does nginx sends 411 when DELETE happens without Content-Length.
If you mix both parts, you could deduce that DELETE should not have any body, and so, should set the Content-Length to 0, like OPTIONS expects.

I can't find any relevent issues on the nginx Trac, I'll try to dig deeper.