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)
Sat, Mar 23, 10:26 PM
Unknown Object (File)
Sun, Mar 10, 10:13 AM
Unknown Object (File)
Feb 13 2024, 11:01 AM
Unknown Object (File)
Feb 3 2024, 5:26 PM
Unknown Object (File)
Feb 2 2024, 3:24 AM
Unknown Object (File)
Jan 18 2024, 6:13 AM
Unknown Object (File)
Dec 27 2023, 12:37 PM
Unknown Object (File)
Dec 27 2023, 12:37 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.