This is an archive of the discontinued Mercurial Phabricator instance.

Differential D3240

httppeer: always add x-hg* headers to Vary header
ClosedPublic

Authored by indygreg on Apr 10 2018, 9:28 PM.

Details

Reviewers
durin42
Group Reviewers
hg-reviewers
Commits
rHG930c433eb311: httppeer: always add x-hg* headers to Vary header
Summary

Before, we manually updated the Vary header value for each
header contributing to it.

All X-Hg* headers are reserved for the Mercurial protocol and
could have caching implications. So it makes sense to always add
these headers to Vary.

A test revealed that X-HgArgs-Post wasn't being added to Vary.
This is only sent on POST requests. POST requests generally
aren't cacheable. However, it is possible if the server sends
the appropriate headers. Mercurial shouldn't be sending those
headers. But let's not take any chances.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

indygreg created this revision.Apr 10 2018, 9:28 PM
durin42 accepted this revision.Apr 11 2018, 12:50 PM
This revision is now accepted and ready to land.Apr 11 2018, 12:50 PM
Closed by commit rHG930c433eb311: httppeer: always add x-hg* headers to Vary header (authored by indygreg). · Explain WhyApr 11 2018, 1:01 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/httppeer.py (8 lines)
Mtests/test-http-bad-server.t (30 lines)

Diff 8001

mercurial/httppeer.py

if cmd == 'pushkey': if cmd == 'pushkey':
args['data'] = '' args['data'] = ''
data = args.pop('data', None) data = args.pop('data', None)
headers = args.pop('headers', {}) headers = args.pop('headers', {})
ui.debug("sending %s command\n" % cmd) ui.debug("sending %s command\n" % cmd)
q = [('cmd', cmd)] q = [('cmd', cmd)]
headersize = 0 headersize = 0
varyheaders = []
# Important: don't use self.capable() here or else you end up # Important: don't use self.capable() here or else you end up
# with infinite recursion when trying to look up capabilities # with infinite recursion when trying to look up capabilities
# for the first time. # for the first time.
postargsok = caps is not None and 'httppostargs' in caps postargsok = caps is not None and 'httppostargs' in caps
# Send arguments via POST. # Send arguments via POST.
if postargsok and args: if postargsok and args:
strargs = urlreq.urlencode(sorted(args.items())) strargs = urlreq.urlencode(sorted(args.items()))
# Send arguments via HTTP headers. # Send arguments via HTTP headers.
if headersize > 0: if headersize > 0:
# The headers can typically carry more data than the URL. # The headers can typically carry more data than the URL.
encargs = urlreq.urlencode(sorted(args.items())) encargs = urlreq.urlencode(sorted(args.items()))
for header, value in encodevalueinheaders(encargs, 'X-HgArg', for header, value in encodevalueinheaders(encargs, 'X-HgArg',
headersize): headersize):
headers[header] = value headers[header] = value
varyheaders.append(header)
# Send arguments via query string (Mercurial <1.9). # Send arguments via query string (Mercurial <1.9).
else: else:
q += sorted(args.items()) q += sorted(args.items())
qs = '?%s' % urlreq.urlencode(q) qs = '?%s' % urlreq.urlencode(q)
cu = "%s%s" % (repobaseurl, qs) cu = "%s%s" % (repobaseurl, qs)
size = 0 size = 0
if util.safehasattr(data, 'length'): if util.safehasattr(data, 'length'):
protoparams.add('comp=%s' % ','.join(comps)) protoparams.add('comp=%s' % ','.join(comps))
if protoparams: if protoparams:
protoheaders = encodevalueinheaders(' '.join(sorted(protoparams)), protoheaders = encodevalueinheaders(' '.join(sorted(protoparams)),
'X-HgProto', 'X-HgProto',
headersize or 1024) headersize or 1024)
for header, value in protoheaders: for header, value in protoheaders:
headers[header] = value headers[header] = value
varyheaders = []
for header in headers:
if header.lower().startswith(r'x-hg'):
varyheaders.append(header) varyheaders.append(header)
if varyheaders: if varyheaders:
headers[r'Vary'] = r','.join(varyheaders) headers[r'Vary'] = r','.join(sorted(varyheaders))
req = requestbuilder(pycompat.strurl(cu), data, headers) req = requestbuilder(pycompat.strurl(cu), data, headers)
if data is not None: if data is not None:
ui.debug("sending %d bytes\n" % size) ui.debug("sending %d bytes\n" % size)
req.add_unredirected_header(r'Content-Length', r'%d' % size) req.add_unredirected_header(r'Content-Length', r'%d' % size)
return req, cu, qs return req, cu, qs

tests/test-http-bad-server.t

readline(247 from -1) -> (29) vary: X-HgArg-1,X-HgProto-1\r\n readline(247 from -1) -> (29) vary: X-HgArg-1,X-HgProto-1\r\n
readline(218 from -1) -> (218) x-hgarg-1: bookmarks=1&bundlecaps=HG20%2Cbundle2%3DHG20%250Abookmarks%250Achangegroup%253D01%252C02%250Adigests%253Dmd5%252Csha1%252Csha512%250Aerror%253Dabort%252Cunsupportedcontent%252Cpushraced%252Cpushkey%250Ahgtag readline(218 from -1) -> (218) x-hgarg-1: bookmarks=1&bundlecaps=HG20%2Cbundle2%3DHG20%250Abookmarks%250Achangegroup%253D01%252C02%250Adigests%253Dmd5%252Csha1%252Csha512%250Aerror%253Dabort%252Cunsupportedcontent%252Cpushraced%252Cpushkey%250Ahgtag
read limit reached; closing socket read limit reached; closing socket
$ rm -f error.log $ rm -f error.log
Now do a variation using POST to send arguments Now do a variation using POST to send arguments
$ hg serve --config experimental.httppostargs=true --config badserver.closeafterrecvbytes=361,330 -p $HGPORT -d --pid-file=hg.pid -E error.log $ hg serve --config experimental.httppostargs=true --config badserver.closeafterrecvbytes=375,344 -p $HGPORT -d --pid-file=hg.pid -E error.log
$ cat hg.pid > $DAEMON_PIDS $ cat hg.pid > $DAEMON_PIDS
$ hg clone http://localhost:$HGPORT/ clone $ hg clone http://localhost:$HGPORT/ clone
abort: error: bad HTTP status line: '' abort: error: bad HTTP status line: ''
[255] [255]
$ killdaemons.py $DAEMON_PIDS $ killdaemons.py $DAEMON_PIDS
$ cat error.log $ cat error.log
readline(361 from 65537) -> (33) GET /?cmd=capabilities HTTP/1.1\r\n readline(375 from 65537) -> (33) GET /?cmd=capabilities HTTP/1.1\r\n
readline(328 from -1) -> (27) Accept-Encoding: identity\r\n readline(342 from -1) -> (27) Accept-Encoding: identity\r\n
readline(301 from -1) -> (19) vary: X-HgProto-1\r\n readline(315 from -1) -> (19) vary: X-HgProto-1\r\n
readline(282 from -1) -> (27) x-hgproto-1: partial-pull\r\n readline(296 from -1) -> (27) x-hgproto-1: partial-pull\r\n
readline(255 from -1) -> (35) accept: application/mercurial-0.1\r\n readline(269 from -1) -> (35) accept: application/mercurial-0.1\r\n
readline(220 from -1) -> (2?) host: localhost:$HGPORT\r\n (glob) readline(234 from -1) -> (2?) host: localhost:$HGPORT\r\n (glob)
readline(* from -1) -> (49) user-agent: mercurial/proto-1.0 (Mercurial 4.2)\r\n (glob) readline(* from -1) -> (49) user-agent: mercurial/proto-1.0 (Mercurial 4.2)\r\n (glob)
readline(* from -1) -> (2) \r\n (glob) readline(* from -1) -> (2) \r\n (glob)
write(36) -> HTTP/1.1 200 Script output follows\r\n write(36) -> HTTP/1.1 200 Script output follows\r\n
write(23) -> Server: badhttpserver\r\n write(23) -> Server: badhttpserver\r\n
write(37) -> Date: $HTTP_DATE$\r\n write(37) -> Date: $HTTP_DATE$\r\n
write(41) -> Content-Type: application/mercurial-0.1\r\n write(41) -> Content-Type: application/mercurial-0.1\r\n
write(21) -> Content-Length: 449\r\n write(21) -> Content-Length: 449\r\n
write(2) -> \r\n write(2) -> \r\n
write(449) -> batch branchmap bundle2=HG20%0Abookmarks%0Achangegroup%3D01%2C02%0Adigests%3Dmd5%2Csha1%2Csha512%0Aerror%3Dabort%2Cunsupportedcontent%2Cpushraced%2Cpushkey%0Ahgtagsfnodes%0Alistkeys%0Apushkey%0Aremote-changegroup%3Dhttp%2Chttps%0Arev-branch-cache changegroupsubset compression=none getbundle httpheader=1024 httpmediatype=0.1rx,0.1tx,0.2tx httppostargs known lookup pushkey streamreqs=generaldelta,revlogv1 unbundle=HG10GZ,HG10BZ,HG10UN unbundlehash write(449) -> batch branchmap bundle2=HG20%0Abookmarks%0Achangegroup%3D01%2C02%0Adigests%3Dmd5%2Csha1%2Csha512%0Aerror%3Dabort%2Cunsupportedcontent%2Cpushraced%2Cpushkey%0Ahgtagsfnodes%0Alistkeys%0Apushkey%0Aremote-changegroup%3Dhttp%2Chttps%0Arev-branch-cache changegroupsubset compression=none getbundle httpheader=1024 httpmediatype=0.1rx,0.1tx,0.2tx httppostargs known lookup pushkey streamreqs=generaldelta,revlogv1 unbundle=HG10GZ,HG10BZ,HG10UN unbundlehash
readline\(14[67] from 65537\) -> \(2[67]\) POST /\?cmd=batch HTTP/1.1\\r\\n (re) readline(1?? from 65537) -> (27) POST /?cmd=batch HTTP/1.1\r\n (glob)
readline\(1(19|20) from -1\) -> \(27\) Accept-Encoding: identity\\r\\n (re) readline(1?? from -1) -> (27) Accept-Encoding: identity\r\n (glob)
readline(9? from -1) -> (41) content-type: application/mercurial-0.1\r\n (glob) readline(1?? from -1) -> (41) content-type: application/mercurial-0.1\r\n (glob)
readline(5? from -1) -> (19) vary: X-HgProto-1\r\n (glob) readline(6? from -1) -> (33) vary: X-HgArgs-Post,X-HgProto-1\r\n (glob)
readline(3? from -1) -> (19) x-hgargs-post: 28\r\n (glob) readline(3? from -1) -> (19) x-hgargs-post: 28\r\n (glob)
readline(1? from -1) -> (1?) x-hgproto-1: * (glob) readline(1? from -1) -> (1?) x-hgproto-1: * (glob)
read limit reached; closing socket read limit reached; closing socket
readline(330 from 65537) -> (27) POST /?cmd=batch HTTP/1.1\r\n readline(344 from 65537) -> (27) POST /?cmd=batch HTTP/1.1\r\n
readline(303 from -1) -> (27) Accept-Encoding: identity\r\n readline(317 from -1) -> (27) Accept-Encoding: identity\r\n
readline(276 from -1) -> (41) content-type: application/mercurial-0.1\r\n readline(290 from -1) -> (41) content-type: application/mercurial-0.1\r\n
readline(235 from -1) -> (19) vary: X-HgProto-1\r\n readline(249 from -1) -> (33) vary: X-HgArgs-Post,X-HgProto-1\r\n
readline(216 from -1) -> (19) x-hgargs-post: 28\r\n readline(216 from -1) -> (19) x-hgargs-post: 28\r\n
readline(197 from -1) -> (61) x-hgproto-1: 0.1 0.2 comp=$USUAL_COMPRESSIONS$ partial-pull\r\n readline(197 from -1) -> (61) x-hgproto-1: 0.1 0.2 comp=$USUAL_COMPRESSIONS$ partial-pull\r\n
readline(136 from -1) -> (35) accept: application/mercurial-0.1\r\n readline(136 from -1) -> (35) accept: application/mercurial-0.1\r\n
readline(101 from -1) -> (20) content-length: 28\r\n readline(101 from -1) -> (20) content-length: 28\r\n
readline(81 from -1) -> (*) host: localhost:$HGPORT\r\n (glob) readline(81 from -1) -> (*) host: localhost:$HGPORT\r\n (glob)
readline(* from -1) -> (49) user-agent: mercurial/proto-1.0 (Mercurial 4.2)\r\n (glob) readline(* from -1) -> (49) user-agent: mercurial/proto-1.0 (Mercurial 4.2)\r\n (glob)
readline(* from -1) -> (2) \r\n (glob) readline(* from -1) -> (2) \r\n (glob)
read(* from 28) -> (*) cmds=* (glob) read(* from 28) -> (*) cmds=* (glob)