Patchwork lfs: explicitly add the Content-Length header when uploading blobs, for py3

login
register
mail settings
Submitter Matt Harbison
Date Jan. 29, 2019, 2:48 a.m.
Message ID <1bc01490178a97d4af90.1548730129@Envy>
Download mbox | patch
Permalink /patch/38148/
State Accepted
Headers show

Comments

Matt Harbison - Jan. 29, 2019, 2:48 a.m.
# HG changeset patch
# User Matt Harbison <matt_harbison@yahoo.com>
# Date 1548729306 18000
#      Mon Jan 28 21:35:06 2019 -0500
# Node ID 1bc01490178a97d4af903c87581bc270362bc079
# Parent  7eb7637e34bf412374a165fbd234022a5a804683
lfs: explicitly add the Content-Length header when uploading blobs, for py3

This was the reason for test-lfs-test-server.t#git-server complaining about an
"invalid byte in chunk length".  For some reason if this isn't explicitly added,
py3.7.1 is adding `transfer-encoding: chunked` as well as `Content-length: x`.
Wireshark flagged this as malformed.  However, if this is set, it doesn't bother
with `transfer-encoding`.

Before this patch with py3:

  PUT /objects/31cf46fbc4ecd458a0943c5b4881f1f5a6dd36c53d6167d5b69ac45149b38e5b HTTP/1.1
  Accept-Encoding: identity
  Content-length: 12
  accept: application/vnd.git-lfs
  content-type: application/octet-stream
  host: localhost:20062
  transfer-encoding: chunked
  user-agent: git-lfs/2.3.4 (Mercurial 4.9rc0+149-7eb7637e34bf)

Before this patch with py27:

  PUT /objects/31cf46fbc4ecd458a0943c5b4881f1f5a6dd36c53d6167d5b69ac45149b38e5b HTTP/1.1
  Accept-Encoding: identity
  accept: application/vnd.git-lfs
  content-type: application/octet-stream
  content-length: 12
  host: localhost:20062
  user-agent: git-lfs/2.3.4 (Mercurial 4.9rc0+149-7eb7637e34bf+20190128)

With this patch and py3, the content is the same as the py27 example.  RFC2616
says to ignore `Content-Length` if `Transfer-Encoding` is present, so maybe
there's nothing to do in the hg-server side (though I'm not sure which it is
using if presented both).

Maybe chunked encoding is better to do?  If someone knows how to suppress the
`Content-Length`, we can try that instead.
Yuya Nishihara - Jan. 29, 2019, 11:03 a.m.
On Mon, 28 Jan 2019 21:48:49 -0500, Matt Harbison wrote:
> # HG changeset patch
> # User Matt Harbison <matt_harbison@yahoo.com>
> # Date 1548729306 18000
> #      Mon Jan 28 21:35:06 2019 -0500
> # Node ID 1bc01490178a97d4af903c87581bc270362bc079
> # Parent  7eb7637e34bf412374a165fbd234022a5a804683
> lfs: explicitly add the Content-Length header when uploading blobs, for py3

Queued, thanks.

Patch

diff --git a/contrib/python3-whitelist b/contrib/python3-whitelist
--- a/contrib/python3-whitelist
+++ b/contrib/python3-whitelist
@@ -352,6 +352,7 @@  test-lfconvert.t
 test-lfs-bundle.t
 test-lfs-largefiles.t
 test-lfs-pointer.py
+test-lfs-test-server.t
 test-lfs.t
 test-linelog.py
 test-linerange.py
diff --git a/hgext/lfs/blobstore.py b/hgext/lfs/blobstore.py
--- a/hgext/lfs/blobstore.py
+++ b/hgext/lfs/blobstore.py
@@ -428,6 +428,7 @@  class _gitlfsremote(object):
             request.data = filewithprogress(localstore.open(oid), None)
             request.get_method = lambda: r'PUT'
             request.add_header(r'Content-Type', r'application/octet-stream')
+            request.add_header(r'Content-Length', len(request.data))
 
         for k, v in headers:
             request.add_header(pycompat.strurl(k), pycompat.strurl(v))