Patchwork [9,of,9,remotefilelog-ext,getfile-http] remotefilelogserver: prevent getfiles from being called over http at all

login
register
mail settings
Submitter Augie Fackler
Date July 1, 2015, 8:05 p.m.
Message ID <f4d310b5f7fc0213ec07.1435781146@arthedain.pit.corp.google.com>
Download mbox | patch
Permalink /patch/9860/
State Not Applicable
Headers show

Comments

Augie Fackler - July 1, 2015, 8:05 p.m.
# HG changeset patch
# User Augie Fackler <augie@google.com>
# Date 1435676687 14400
#      Tue Jun 30 11:04:47 2015 -0400
# Node ID f4d310b5f7fc0213ec07435802e594b807b260f4
# Parent  99e8e62032be03b27e450cfb261a5cd61a960360
remotefilelogserver: prevent getfiles from being called over http at all

This means that even old clients that fail to sniff for capabilities
before trying getfiles will get a sensible error message back from the
server.
Durham Goode - July 2, 2015, 3:18 a.m.
Series looks good.  I'll push it.

Only change I made is that you use get-with-headers in the tests, but it 
doesn't exist in remotefilelog/tests.  I'm guessing you're running with 
the upstream run-tests.py?  I'll just add that file to tests/

On 7/1/15 1:05 PM, Augie Fackler wrote:
> # HG changeset patch
> # User Augie Fackler <augie@google.com>
> # Date 1435676687 14400
> #      Tue Jun 30 11:04:47 2015 -0400
> # Node ID f4d310b5f7fc0213ec07435802e594b807b260f4
> # Parent  99e8e62032be03b27e450cfb261a5cd61a960360
> remotefilelogserver: prevent getfiles from being called over http at all
>
> This means that even old clients that fail to sniff for capabilities
> before trying getfiles will get a sensible error message back from the
> server.
>
> diff --git a/remotefilelog/remotefilelogserver.py b/remotefilelog/remotefilelogserver.py
> --- a/remotefilelog/remotefilelogserver.py
> +++ b/remotefilelog/remotefilelogserver.py
> @@ -8,6 +8,7 @@
>   from mercurial import wireproto, changegroup, match, util, changelog, context
>   from mercurial import exchange, sshserver
>   from mercurial.extensions import wrapfunction
> +from mercurial.hgweb import protocol as httpprotocol
>   from mercurial.node import bin, hex, nullid, nullrev
>   from mercurial.i18n import _
>   import shallowrepo
> @@ -170,6 +171,13 @@ def onetimesetup(ui):
>   
>       wrapfunction(context.basefilectx, '_adjustlinkrev', _adjustlinkrev)
>   
> +    def _iscmd(orig, cmd):
> +        if cmd == 'getfiles':
> +            return False
> +        return orig(cmd)
> +
> +    wrapfunction(httpprotocol, 'iscmd', _iscmd)
> +
>   def getfiles(repo, proto):
>       """A server api for requesting particular versions of particular files.
>       """
> diff --git a/tests/test-http.t b/tests/test-http.t
> --- a/tests/test-http.t
> +++ b/tests/test-http.t
> @@ -31,6 +31,5 @@ as the getfile method it offers doesn't
>   
>     $ get-with-headers.py localhost:$HGPORT '?cmd=this-command-does-not-exist' | head -n 1
>     400 no such method: this-command-does-not-exist
> -Bug: this should return 400, not 5xx
>     $ get-with-headers.py localhost:$HGPORT '?cmd=getfiles' | head -n 1
> -  500 Internal Server Error
> +  400 no such method: getfiles
Augie Fackler - July 2, 2015, 3:26 a.m.
On Jul 1, 2015 11:18 PM, "Durham Goode" <durham@fb.com> wrote:
>
> Series looks good.  I'll push it.
>
> Only change I made is that you use get-with-headers in the tests, but it
doesn't exist in remotefilelog/tests.  I'm guessing you're running with the
upstream run-tests.py?  I'll just add that file to tests/

Oh, yup. Was using the one from upstream so I could iterate on wireproto
silliness. Thanks!

>
>
> On 7/1/15 1:05 PM, Augie Fackler wrote:
>>
>> # HG changeset patch
>> # User Augie Fackler <augie@google.com>
>> # Date 1435676687 14400
>> #      Tue Jun 30 11:04:47 2015 -0400
>> # Node ID f4d310b5f7fc0213ec07435802e594b807b260f4
>> # Parent  99e8e62032be03b27e450cfb261a5cd61a960360
>> remotefilelogserver: prevent getfiles from being called over http at all
>>
>> This means that even old clients that fail to sniff for capabilities
>> before trying getfiles will get a sensible error message back from the
>> server.
>>
>> diff --git a/remotefilelog/remotefilelogserver.py
b/remotefilelog/remotefilelogserver.py
>> --- a/remotefilelog/remotefilelogserver.py
>> +++ b/remotefilelog/remotefilelogserver.py
>> @@ -8,6 +8,7 @@
>>   from mercurial import wireproto, changegroup, match, util, changelog,
context
>>   from mercurial import exchange, sshserver
>>   from mercurial.extensions import wrapfunction
>> +from mercurial.hgweb import protocol as httpprotocol
>>   from mercurial.node import bin, hex, nullid, nullrev
>>   from mercurial.i18n import _
>>   import shallowrepo
>> @@ -170,6 +171,13 @@ def onetimesetup(ui):
>>         wrapfunction(context.basefilectx, '_adjustlinkrev',
_adjustlinkrev)
>>   +    def _iscmd(orig, cmd):
>> +        if cmd == 'getfiles':
>> +            return False
>> +        return orig(cmd)
>> +
>> +    wrapfunction(httpprotocol, 'iscmd', _iscmd)
>> +
>>   def getfiles(repo, proto):
>>       """A server api for requesting particular versions of particular
files.
>>       """
>> diff --git a/tests/test-http.t b/tests/test-http.t
>> --- a/tests/test-http.t
>> +++ b/tests/test-http.t
>> @@ -31,6 +31,5 @@ as the getfile method it offers doesn't
>>       $ get-with-headers.py localhost:$HGPORT
'?cmd=this-command-does-not-exist' | head -n 1
>>     400 no such method: this-command-does-not-exist
>> -Bug: this should return 400, not 5xx
>>     $ get-with-headers.py localhost:$HGPORT '?cmd=getfiles' | head -n 1
>> -  500 Internal Server Error
>> +  400 no such method: getfiles
>
>

Patch

diff --git a/remotefilelog/remotefilelogserver.py b/remotefilelog/remotefilelogserver.py
--- a/remotefilelog/remotefilelogserver.py
+++ b/remotefilelog/remotefilelogserver.py
@@ -8,6 +8,7 @@ 
 from mercurial import wireproto, changegroup, match, util, changelog, context
 from mercurial import exchange, sshserver
 from mercurial.extensions import wrapfunction
+from mercurial.hgweb import protocol as httpprotocol
 from mercurial.node import bin, hex, nullid, nullrev
 from mercurial.i18n import _
 import shallowrepo
@@ -170,6 +171,13 @@  def onetimesetup(ui):
 
     wrapfunction(context.basefilectx, '_adjustlinkrev', _adjustlinkrev)
 
+    def _iscmd(orig, cmd):
+        if cmd == 'getfiles':
+            return False
+        return orig(cmd)
+
+    wrapfunction(httpprotocol, 'iscmd', _iscmd)
+
 def getfiles(repo, proto):
     """A server api for requesting particular versions of particular files.
     """
diff --git a/tests/test-http.t b/tests/test-http.t
--- a/tests/test-http.t
+++ b/tests/test-http.t
@@ -31,6 +31,5 @@  as the getfile method it offers doesn't 
 
   $ get-with-headers.py localhost:$HGPORT '?cmd=this-command-does-not-exist' | head -n 1
   400 no such method: this-command-does-not-exist
-Bug: this should return 400, not 5xx
   $ get-with-headers.py localhost:$HGPORT '?cmd=getfiles' | head -n 1
-  500 Internal Server Error
+  400 no such method: getfiles