empty body in post ClientRequest does not populate Content-Length

[robjtede]

Whist implementing a google recaptcha backend I was getting HTTP 411 (Length Required) error responses from Google using roughly this code.

ClientRequest::post(recaptcha_url)
  .finish()
  .unwrap()
  .send();

and even adding a content-length manually still produced a 411. Eg.

ClientRequest::post(recaptcha_url)
  .content_length(0)
  .finish()
  .unwrap()
  .send();

A debug output of the request showed the content-length in the header section but a Wireshark dump of the outgoing request showed no such header.

Req/Res Debug Output	Wireshark Log

This is the code I had to use:

ClientRequest::post(recaptcha_url)
  .body("")
  .unwrap()
  .send();

Obviously a fine workaround but this shows that a default content-length of 0 wasnt getting set and that the debug output was mis-reporting it.

actix = "0.7"
actix-web = { version = "0.7", features = ["ssl"] }

[DoumanAsh]

If user specified length 0 then we should add header

P.s. I think we are supposed not to set length unless user specifies empty body, or manually set length

[robjtede]

I think we are supposed not to set length unless user specifies empty body, or manually set length

The implementation of .finish() sets .body to Body::Empty and the docs for Body say under the Body::Empty variant "Empty response. Content-Length header is set to 0"

[DoumanAsh]

True, looking at code for H1 writer, we're supposed to insert Content-Length unless you manually set it... Not sure how it is possible that you're actually missing it, guess I'll need to try to add some logs and see it for myself

[DoumanAsh]

@robjtede I assume the intent here is that while for server response writing 0 len makes sense always, for client it might not be so. I made PR and you can test it if you want, but I'm thinking whether we should do it only for requests with content like PUT/POST

[robjtede]

Yeah this request specifically was a post request without a body, the submitted information was in the url parameters. I’ll check out the pr on Monday.

[DoumanAsh]

@robjtede ping

[robjtede]

PR looks good.

Feels like DELETE and PATCH should be included if PUT is since they often carry payloads, too.

[DoumanAsh]

@robjtede Ok, I can understand PATCH but I do not see how DELETE is supposed to carry payload...?

[robjtede]

The spec does not disallow or discourage it like it does with GET/HEAD/OPTIONS

Edit: further reading into other recommended REST implementations say that any body passed with a DELETE should be ignored. Makes sense but I have used it in a rare case at work.

Probably just leave it out; at least for now.

[DoumanAsh]

Yeah, it doesn't disallow, but RFC specifies that we must add zero length only for verbs whose semantics require payload which is not the case for DELETE.

I'll add it only for PATCH