Clone failure on android.googlesource.com repos #2518
Comments
|
The size of that buffer is slightly larger than the maximum packet size as defined by the Git Smart Protocol[1] so there should never be an issue filling it. My first impression is that It looks like whatever is serving the android repositories might be trying to send more data than it's allowed, though it's odd that git wouldn't face the same issue. Annoyingly, this host only seems to be available through HTTPS which makes capturing the output a PITA. [1] https://github.com/git/git/blob/master/Documentation/technical/protocol-capabilities.txt#L118-L151 |
|
The good new is that gerrit is not breaking the protocol. The issue seems to be some bad combination of recv timing on our http parser side. The http parser pushes data into our buffer, and I'm seeing two successive pushes, one of which is just over the size we have left in our buffer. It looks like we're receiving the end of one HTTP chunk which coincides with the end of the pkt, but our recv buffer has the start of the next chunk, so http-parser sends that as well, which would overflow our buffer. Making the buffer larger is one way to make sure this doesn't happen, but it's probably not the most correct one. I think that if we make the recv buffer ( |
The recv buffer (parse_buffer) and the buffer have independent sizes and offsets. We try to fill in parse_buffer as much as possible before passing it to the http parser. This is fine most of the time, but fails us when the buffer is almost full. In those situations, parse_buffer can have more data than we would be able to put into the buffer (which may be getting full if we're towards the end of a data sideband packet). To work around this, we check if the space we have left on our buffer is smaller than what could come from the network. If this happens, we make parse_buffer think that it has as much space left as our buffer, so it won't try to retrieve more data than we can deal with. As the start of the data may no longer be at the start of the buffer, we need to keep track of where it really starts (data_offset) and use that in our calculations for the real size of the data we received from the network. This fixes #2518.
The recv buffer (parse_buffer) and the buffer have independent sizes and offsets. We try to fill in parse_buffer as much as possible before passing it to the http parser. This is fine most of the time, but fails us when the buffer is almost full. In those situations, parse_buffer can have more data than we would be able to put into the buffer (which may be getting full if we're towards the end of a data sideband packet). To work around this, we check if the space we have left on our buffer is smaller than what could come from the network. If this happens, we make parse_buffer think that it has as much space left as our buffer, so it won't try to retrieve more data than we can deal with. As the start of the data may no longer be at the start of the buffer, we need to keep track of where it really starts (data_offset) and use that in our calculations for the real size of the data we received from the network. This fixes #2518.
|
Any chance we can merge this change in the maintenance release? I was hoping it would be v0.21.2. If there is going to be a v0.21.3 could this be included please? (Sorry couldn't find the policy of merging change into maintenance releases but this seem like a genuine bug that effects anybody pulling from googlesource repositories). |
|
This was included in the maintenance branch as 7d729d0 which is included in v0.21.2. |
This fixes the issue of accessing googlesource.com repos: libgit2/libgit2#2518 R=bradnelson@google.com Review URL: https://codereview.chromium.org/744243002
Using libgit2 to clone any of the repositories here:
https://android.googlesource.com/
fails with
Can't fit data in the buffer(at transports/http.c:L386):The buffer is
transport_smart.buffer_dataintransports/smart.h. I tried the naive solution and bumped the size of this buffer to 262144, which made the problem go away. I don't understand the protocol well enough to know what's wrong though - I expect there is a nicer fix than that?The text was updated successfully, but these errors were encountered: