fix request with content-length > 0 and content-encoding: gzip

[seangarner]

The Test class wasn't constructing the Request instance with an uppercase method as it expects (e.g. here, here and here).

This results in a number of edge case bugs including on node 6 when a HEAD response has a content-encoding response of gzip and there is also a content-length > 0. superagent would callback Error: unexpected end of file. Superagent correctly interprets the http spec by ignoring the content-length and response body of a HEAD request.

[kornelski]

Oh. Technically, methods are case-sensitive:

http://httpwg.org/specs/rfc7231.html#method.overview

The method token is case-sensitive because it might be used as a gateway to object-based systems with case-sensitive method names.

[seangarner]

Then maybe supertest needs to expose uppercase methods for GET, HEAD, POST,
etc?

On Wed, 31 Aug 2016, 21:50 Kornel, [email protected] wrote:

Oh. Technically, methods are case-sensitive:

http://httpwg.org/specs/rfc7231.html#method.overview

The method token is case-sensitive because it might be used as a gateway
to object-based systems with case-sensitive method names.

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#371 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABsqBq7WtbY5ohi0yEW1L19PJX1Btl54ks5qlekFgaJpZM4Jx7E-
.

[kornelski]

Yeah, uppercasing known method types (and we have a list in the methods module) sounds like a good compromise.

[seangarner]

I left this over night to think on it because my suggestion didn't sit well with me. Whilst the RFC clearly states methods are case sensitive there's an awful lot of prior art in the node ecosystem that ignores that fact:

• express has lowercase functions for routing of uppercase HTTP methods https://expressjs.com/en/4x/api.html#app.METHOD
• ...as does restify http://restify.com/#routing
• even node core does it: https://nodejs.org/docs/latest-argon/api/http.html#http_http_get_options_callback
• and other request libraries like https://www.npmjs.com/package/request
• not to mention superagent itself

I'd also be concerned that developers would unknowingly continue to use the lowercase methods without understanding the edge cases.

Thoughts @pornel?

[kornelski]

I see there's a convention of lowercasing HTTP method names when exposing them as JS method (function) names, but req.method still returns an uppercase name, so purely from API perspective it still seems like it could support case-sensitivity, at least for custom methods.

However, I've found that Node's http module doesn't support receiving requests with custom HTTP methods at all. That's a strong case against bothering to support unusual HTTP method names. I can't even write a test for sending a custom method name correctly if Node can't receive it.

So I'm OK with ignoring case-sensitivity of HTTP method names, and blame it on Node breaking it first ;)