From 13c8963dab27b5ee24a87c5f4ebbf18be585dc19 Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 4 Jan 2018 13:41:20 -0800 Subject: [PATCH] Fix a Mercurial wire protocol parser issue when we receive a length frame before any data Summary: Depends on D18856. Ref T13036. See PHI275. When we receive a length frame but the buffer doesn't have any data yet, we currently emit a pointless 0-length data frame on the channel. For normal chatter this is harmless/valid, but it causes problems when a channel has transitioned into bundle2 mode (probably it indicates "end of stream")? In any case, it's never helpful, so if we're about to read a data block and don't have any data, just bail out until we see some more data. Note that we can't end up here //expecting// a 0-length data block: both the `data-length` and `data-bytes` states already handle that properly. Test Plan: Pushed 4MB changes to a Mercurial repository with Mercurial 4.1.1, was no longer able to hit channel errors. Reviewers: amckinley Reviewed By: amckinley Maniphest Tasks: T13036 Differential Revision: https://secure.phabricator.com/D18857 --- .../ssh/DiffusionMercurialWireClientSSHProtocolChannel.php | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/applications/diffusion/ssh/DiffusionMercurialWireClientSSHProtocolChannel.php b/src/applications/diffusion/ssh/DiffusionMercurialWireClientSSHProtocolChannel.php index 5150ef9352..4c16fb9f0f 100644 --- a/src/applications/diffusion/ssh/DiffusionMercurialWireClientSSHProtocolChannel.php +++ b/src/applications/diffusion/ssh/DiffusionMercurialWireClientSSHProtocolChannel.php @@ -192,6 +192,13 @@ final class DiffusionMercurialWireClientSSHProtocolChannel $this->state = 'data-bytes'; } } else if ($this->state == 'data-bytes') { + // If we don't have any more bytes on the buffer yet, just bail: + // otherwise, we'll emit a pointless and possibly harmful 0-byte data + // frame. See T13036 for discussion. + if (!strlen($this->buffer)) { + break; + } + $bytes = substr($this->buffer, 0, $this->expectBytes); $this->buffer = substr($this->buffer, strlen($bytes)); $this->expectBytes -= strlen($bytes);