diff --git a/src/applications/diffusion/protocol/DiffusionSubversionWireProtocol.php b/src/applications/diffusion/protocol/DiffusionSubversionWireProtocol.php index 1cb0f107aa..251967f314 100644 --- a/src/applications/diffusion/protocol/DiffusionSubversionWireProtocol.php +++ b/src/applications/diffusion/protocol/DiffusionSubversionWireProtocol.php @@ -33,7 +33,24 @@ final class DiffusionSubversionWireProtocol extends Phobject { $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 @@ final class DiffusionSubversionWireProtocol extends Phobject { ); $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 @@ final class DiffusionSubversionWireProtocol extends Phobject { // 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 index bd99b82035..f4309b0e7b 100644 --- a/src/applications/diffusion/protocol/__tests__/DiffusionSubversionWireProtocolTestCase.php +++ b/src/applications/diffusion/protocol/__tests__/DiffusionSubversionWireProtocolTestCase.php @@ -59,6 +59,50 @@ final class DiffusionSubversionWireProtocolTestCase ), ), )); + + // 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 @@ final class DiffusionSubversionWireProtocolTestCase 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 @@ final class DiffusionSubversionWireProtocolTestCase $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.'>'); } }