Page MenuHomePhabricator
Files F15525575

D19451.id46532.diff
No OneTemporary
Actions

D19451.id46532.diff
View Options

diff --git a/src/applications/diffusion/protocol/DiffusionSubversionWireProtocol.php b/src/applications/diffusion/protocol/DiffusionSubversionWireProtocol.php
--- a/src/applications/diffusion/protocol/DiffusionSubversionWireProtocol.php
+++ b/src/applications/diffusion/protocol/DiffusionSubversionWireProtocol.php
@@ -33,7 +33,24 @@
$messages = array();
while (true) {
- if ($this->state == 'item') {
+ if ($this->state == 'space') {
+ // Consume zero or more extra spaces after matching an item. The
+ // protocol requires at least one space, but allows more than one.
+
+ $matches = null;
+ if (!preg_match('/^(\s*)\S/', $this->buffer, $matches)) {
+ // Wait for more data.
+ break;
+ }
+
+ // We have zero or more spaces and then some other character, so throw
+ // the spaces away and continue parsing frames.
+ if (strlen($matches[1])) {
+ $this->buffer = substr($this->buffer, strlen($matches[1]));
+ }
+
+ $this->state = 'item';
+ } else if ($this->state == 'item') {
$match = null;
$result = null;
$buf = $this->buffer;
@@ -69,6 +86,12 @@
);
$this->raw = '';
}
+
+ // Consume any extra whitespace after an item. If we're in the
+ // "bytes" state, we aren't looking for whitespace.
+ if ($this->state == 'item') {
+ $this->state = 'space';
+ }
} else {
// No matches yet, wait for more data.
break;
@@ -90,7 +113,7 @@
// Strip off the terminal space.
$this->pushItem(substr($this->byteBuffer, 0, -1), 'string');
$this->byteBuffer = '';
- $this->state = 'item';
+ $this->state = 'space';
}
} else {
throw new Exception(pht("Invalid state '%s'!", $this->state));
diff --git a/src/applications/diffusion/protocol/__tests__/DiffusionSubversionWireProtocolTestCase.php b/src/applications/diffusion/protocol/__tests__/DiffusionSubversionWireProtocolTestCase.php
--- a/src/applications/diffusion/protocol/__tests__/DiffusionSubversionWireProtocolTestCase.php
+++ b/src/applications/diffusion/protocol/__tests__/DiffusionSubversionWireProtocolTestCase.php
@@ -59,6 +59,50 @@
),
),
));
+
+ // This is testing that multiple spaces are parsed correctly. See T13140
+ // for discussion.
+ $this->assertSameSubversionMessages(
+ '( get-file true false ) ',
+ // ^-- Note extra space!
+ array(
+ array(
+ array(
+ 'type' => 'word',
+ 'value' => 'get-file',
+ ),
+ array(
+ 'type' => 'word',
+ 'value' => 'true',
+ ),
+ array(
+ 'type' => 'word',
+ 'value' => 'false',
+ ),
+ ),
+ ),
+ '( get-file true false ) ');
+
+ $this->assertSameSubversionMessages(
+ '( duck 5:quack moo ) ',
+ array(
+ array(
+ array(
+ 'type' => 'word',
+ 'value' => 'duck',
+ ),
+ array(
+ 'type' => 'string',
+ 'value' => 'quack',
+ ),
+ array(
+ 'type' => 'word',
+ 'value' => 'moo',
+ ),
+ ),
+ ),
+ '( duck 5:quack moo ) ');
+
}
public function testSubversionWireProtocolPartialFrame() {
@@ -86,7 +130,11 @@
ipull($msg2, 'structure'));
}
- private function assertSameSubversionMessages($string, array $structs) {
+ private function assertSameSubversionMessages(
+ $string,
+ array $structs,
+ $serial_string = null) {
+
$proto = new DiffusionSubversionWireProtocol();
// Verify that the wire message parses into the structs.
@@ -100,6 +148,13 @@
$serial[] = $proto->serializeStruct($struct);
}
$serial = implode('', $serial);
- $this->assertEqual($string, $serial, 'serialize<'.$string.'>');
+
+ if ($serial_string === null) {
+ $expect_serial = $string;
+ } else {
+ $expect_serial = $serial_string;
+ }
+
+ $this->assertEqual($expect_serial, $serial, 'serialize<'.$string.'>');
}
}

File Metadata

Mime Type
text/plain
Expires
Tue, Apr 22, 3:24 PM (3 d, 22 h ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7226045
Default Alt Text
D19451.id46532.diff (4 KB)

Event Timeline

Phabricator · Built by Phacility