D3433: httppeer: detect redirect to URL without query string (issue5860)

mharbison72 (Matt Harbison) phabricator at mercurial-scm.org
Fri May 4 23:33:43 EDT 2018


mharbison72 added inline comments.

INLINE COMMENTS

> indygreg wrote in test-http-protocol.t:361
> The distinction between these URLs is that one uses the host:port as it actually is and the other uses the host:port as identified by the client. Since we are sending a URL back to the client, we should be using the advertised host:port.
> 
> The fact that the advertised host:port is being mangled seemingly points to a bug with the formulation of the `advertised*` variables. This is likely the result of code somewhere using `gethostname()` to populate a hostname field. I'm not sure if this is happening on the client or server. And I'm not sure why it would only be happening on Windows.
> 
> It is something to keep our eye on. But I think it is an issue in the test harness and not the hgweb code. We have unit test coverage for URL formulation in `test-wsgirequest.py` and I'm pretty confident it adheres to PEP 3333.

> The fact that the advertised host:port is being mangled seemingly points to a bug
>  with the formulation of the advertised* variables. This is likely the result of code
>  somewhere using gethostname() to populate a hostname field. I'm not sure if this
>  is happening on the client or server. And I'm not sure why it would only be
>  happening on Windows.

The plot thickens...

I stripped down the *.t, ran it with --keep, and then launched a non-daemon `hg serve` + client from the leftover repo, without the test harness.  If I use baseurl, it succeeds as expected, using 127.0.0.1.  If I use advertisedbaseurl, it redirects to the hostname, and the access log shows the external IP address.  So it seems like more than a test harness issue.  (Though in the test harness, this fails with a connection refused.  But ProcessExplorer says it is listening on 0.0.0.0.)

There aren't too many uses of 'gethostname' (insensitive) in python 2.7.15, and none of them seem like an obvious problem.  None of the hits in hg code are relevant.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D3433

To: indygreg, #hg-reviewers
Cc: yuja, martinvonz, mharbison72, mercurial-devel


More information about the Mercurial-devel mailing list