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
Unknown Object (File)
Tue, Oct 4, 3:01 AM
Unknown Object (File)
Sun, Oct 2, 12:09 AM
Unknown Object (File)
Sat, Oct 1, 6:34 AM
Unknown Object (File)
Fri, Sep 30, 3:22 AM
Unknown Object (File)
Thu, Sep 29, 9:12 PM
Unknown Object (File)
Wed, Sep 28, 1:36 AM
Unknown Object (File)
Tue, Sep 27, 7:32 PM
Unknown Object (File)
Sun, Sep 25, 11:53 PM
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
Branch
hg3
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 19015
Build 25651: Run Core Tests
Build 25650: arc lint + arc unit

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.