Odd httpheader=1024 required for Phabricator

[Augie Fackler]

> On Feb 11, 2020, at 16:32, Makarius <makarius at sketis.net> wrote:
> 
> On 11/02/2020 03:20, Augie Fackler wrote:
>> I guess I'm not sure what's going on here. https://www.mercurial-scm.org/repo/hg/rev/5cda0ce05c42 is the revision that introduced that, but I'm not sure why you need to do anything /to phabricator/ unless it's trying (poorly) to pretend to be an hg server. Is it not just blindly proxying the hg protocol from your hg binary?
> 
> Thanks. This hint has helped me to look in the right spots.
> 
> Phabricator essentially starts a command-line process to learn about the
> server capabilities like this:
> 
>  echo capabilities | hg -R .../test-repo serve --stdio
> 
> It uses the result for its own http communication. In the output above, there
> used to be httpheader=1024 until 4.0.2, but it has disappeared in 4.1.
> 
> 
> See also the Phabricator sources:
> 
> https://github.com/phacility/phabricator/blob/master/src/applications/diffusion/controller/DiffusionServeController.php#L781
> 
> https://github.com/phacility/phabricator/blob/master/src/applications/diffusion/controller/DiffusionServeController.php#L838
> 
> https://github.com/phacility/phabricator/blob/master/src/applications/diffusion/protocol/DiffusionMercurialWireProtocol.php#L107
> 
> The latter contains further comments about slightly odd censorship of the
> capabilities. This is also the place where the workaround is inserted, see
> again https://isabelle-dev.sketis.net/T8
> 
> 
> A bisection over the hg repository yields the following relevant changeset:
> 
> changeset:   30563:e118233172fe
> user:        Gregory Szorc <gregory.szorc at gmail.com>
> date:        Mon Nov 28 20:46:42 2016 -0800
> files:       mercurial/wireproto.py tests/test-ssh-bundle1.t tests/test-ssh.t
> description:
> wireproto: only advertise HTTP-specific capabilities to HTTP peers (BC)
> 
> Previously, the capabilities list was protocol agnostic and we
> advertised the same capabilities list to all clients, regardless of
> transport protocol.
> 
> A few capabilities are specific to HTTP. I see no good reason why we
> should advertise them to SSH clients. So this patch limits their
> advertisement to HTTP clients.
> 
> This patch is BC, but SSH clients shouldn't be using the removed
> capabilities so there should be no impact.
> 
> 
> What means "BC"?

"breaking change" or "behavior change" depending who you ask. :)

> Maybe Phabricator is a good reason to keep the full information?

Not especially. They're doing it wrong assuming that the --stdio protocol is compatible with http, and we've told them that more than once and they've responded...negatively. If someone wanted to try and figure out a way to have phabricator do the right thing (invoke hg's WSGI application, probably via CGI?) that would probably help. I'd certainly be open to mentoring someone doing that work, but I can't justify spending time on it myself.

> Do you think you can refine that for a future release of Mercurial?

I'm not sure what you're asking for here...

> 
> In contrast, it is probably difficult to get a patch accepted by the
> Phabricator project, because they are only using rather old hg 2.8.2 for their
> main installation, and 2.6.2, 3.5.1 in their tests:
> https://github.com/phacility/phabricator/blob/master/src/applications/diffusion/protocol/__tests__/DiffusionMercurialWireProtocolTests.php
> 
> 
> 	Makarius