Page MenuHomePhabricator

Fix a Mercurial wire protocol parser issue when we receive a length frame before any data
ClosedPublic

Authored by epriestley on Jan 4 2018, 9:46 PM.
Tags
None
Referenced Files
F15481139: D18857.id.diff
Tue, Apr 8, 6:29 PM
F15455293: D18857.id45237.diff
Sat, Mar 29, 11:23 PM
F15454419: D18857.id45232.diff
Sat, Mar 29, 6:26 PM
F15449937: D18857.id.diff
Fri, Mar 28, 1:01 PM
F15448794: D18857.id45233.diff
Fri, Mar 28, 6:57 AM
F15447190: D18857.diff
Thu, Mar 27, 10:23 PM
F15439657: D18857.id45233.diff
Wed, Mar 26, 8:35 AM
F15438646: D18857.id45233.diff
Wed, Mar 26, 2:17 AM
Subscribers
None

Details

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.

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

epriestley created this revision.
  • Narrower explanatory comment.
amckinley added inline comments.
src/applications/diffusion/ssh/DiffusionMercurialWireClientSSHProtocolChannel.php
182

Not a huge deal, but for consistency with the other branches in this function, it would be nice to have a comment block here.

This revision is now accepted and ready to land.Jan 4 2018, 10:00 PM
This revision was automatically updated to reflect the committed changes.