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
F14067944: D18857.diff
Tue, Nov 19, 6:13 PM
F14055459: D18857.diff
Sat, Nov 16, 12:13 PM
F14041592: D18857.diff
Mon, Nov 11, 7:52 PM
F14027093: D18857.diff
Fri, Nov 8, 5:06 AM
Unknown Object (File)
Oct 9 2024, 8:24 AM
Unknown Object (File)
Oct 8 2024, 9:37 PM
Unknown Object (File)
Sep 20 2024, 8:15 PM
Unknown Object (File)
Sep 12 2024, 9:05 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.