This is an archive of the discontinued Mercurial Phabricator instance.

hgweb: when no agreement on compression can be found, fail for v2
AbandonedPublic

Authored by joerg.sonnenberger on Jan 12 2018, 3:04 AM.

Details

Reviewers
indygreg
Group Reviewers
hg-reviewers
Summary

When the client supports v2 responses, the fallback to the legacy
response is undesirable. If the zlib is acceptable for the client,
it should have said so in first place.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

indygreg requested changes to this revision.Jan 12 2018, 8:20 PM
indygreg added a subscriber: indygreg.

We're not using the Accept* HTTP headers, so the use of HTTP 406 is not appropriate. See https://tools.ietf.org/html/rfc7231#section-6.5.6. I believe if you scour the mailing list or even the commit messages, you'll find text explaining why we explicitly chose to not use the Accept headers for protocol negotiation.

I do think I'm OK with aborting the request if the client request is "malformed." Although it's a BC break and needs to be called out as such. Since we already shipped forgiving code, others may insist we keep the existing behavior. They have a point.

This revision now requires changes to proceed.Jan 12 2018, 8:20 PM

Which status code shall we use then, just plain 400?

Which status code shall we use then, just plain 400?

Good question.

We use 400 for parts of hgweb. But not the wire protocol parts. And the use of 400 should arguably be avoided because it is supposed to mean the HTTP request message itself was malformed. A lot of people (including our uses in hgweb) extend 400 to mean things like the query string parameters are invalid. RFC 7231 is still vague in its wording and does seem to allow liberal use of this status code. That's probably because of common use of 400 in the wild.

In the wire protocol today, it looks like we use 200 and a Content-Type: application/hg-error to indicate error. I think this is what we should use here (assuming existing client codes handles the error sanely). It should, since httppeer.py always checks for this content-type as part of processing every HTTP response.

For the next submission, please add a test showing that an hg operation hitting this error in the context of hg pull or hg clone does something reasonable.