Patchwork [1,of,2,V3] lfs: add git to the User-Agent header for blob transfers

login
register
mail settings
Submitter Matt Harbison
Date Dec. 14, 2017, 6:53 p.m.
Message ID <5482c3c48da7a5e8050a.1513277628@MATT7H-PC.attotech.com>
Download mbox | patch
Permalink /patch/26282/
State Accepted
Headers show

Comments

Matt Harbison - Dec. 14, 2017, 6:53 p.m.
# HG changeset patch
# User Matt Harbison <matt_harbison@yahoo.com>
# Date 1513274648 18000
#      Thu Dec 14 13:04:08 2017 -0500
# Node ID 5482c3c48da7a5e8050aa0a39670a4ce3e5f8a79
# Parent  4937db58b663faa6893c51a41cec28114a165dd0
lfs: add git to the User-Agent header for blob transfers

As we were trying to transition off of the non production lfs-test-server for
further experimenting, one of the problems we ran into was interoperability.  A
coworker setup gitbucket[1] to act as the blob server, tested with git, and
passed it off to me.  But push failed with a message saying "abort: LFS server
returns invalid JSON:", and then proceeded to dump a huge HTML page to the
screen.  It turns out that it is assuming that git is the only thing that wants
to do a blob transfer, and everything else is a web browser wanting HTML.

It's only a single data point, but I suspect other things may be doing this too.
RFC7231 gives an example [2] of listing multiple products in decreasing order of
significance.  Since the standard provides for this, and since it works with the
one problematic server I found, I'm just enabling this by default for a better
UX.

There's nothing significant about the version of git chosen, other than it is
the current version.

[1] https://github.com/gitbucket/gitbucket/
[2] https://tools.ietf.org/html/rfc7231#page-46

Patch

diff --git a/hgext/lfs/blobstore.py b/hgext/lfs/blobstore.py
--- a/hgext/lfs/blobstore.py
+++ b/hgext/lfs/blobstore.py
@@ -101,7 +101,8 @@  class _gitlfsremote(object):
         self.ui = ui
         baseurl, authinfo = url.authinfo()
         self.baseurl = baseurl.rstrip('/')
-        self.urlopener = urlmod.opener(ui, authinfo)
+        useragent = 'mercurial/%s git/2.15.1' % util.version()
+        self.urlopener = urlmod.opener(ui, authinfo, useragent)
         self.retry = ui.configint('lfs', 'retry')
 
     def writebatch(self, pointers, fromstore):
diff --git a/mercurial/url.py b/mercurial/url.py
--- a/mercurial/url.py
+++ b/mercurial/url.py
@@ -466,7 +466,7 @@  class cookiehandler(urlreq.basehandler):
 
 handlerfuncs = []
 
-def opener(ui, authinfo=None):
+def opener(ui, authinfo=None, useragent=None):
     '''
     construct an opener suitable for urllib2
     authinfo will be added to the password manager
@@ -512,8 +512,14 @@  def opener(ui, authinfo=None):
     # own distribution name. Since servers should not be using the user
     # agent string for anything, clients should be able to define whatever
     # user agent they deem appropriate.
-    agent = 'mercurial/proto-1.0 (Mercurial %s)' % util.version()
-    opener.addheaders = [(r'User-agent', pycompat.sysstr(agent))]
+    #
+    # The custom user agent is for lfs, because unfortunately some servers
+    # do look at this value.
+    if not useragent:
+        agent = 'mercurial/proto-1.0 (Mercurial %s)' % util.version()
+        opener.addheaders = [(r'User-agent', pycompat.sysstr(agent))]
+    else:
+        opener.addheaders = [(r'User-agent', pycompat.sysstr(useragent))]
 
     # This header should only be needed by wire protocol requests. But it has
     # been sent on all requests since forever. We keep sending it for backwards