@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.)
hq.recaptime.dev/wiki/Phorge
phorge
phabricator
Parse unusual Subversion protocol frames which contain extra whitespace
Summary:
Fixes T13140. See PHI660.
Recent versions of Subversion can send a `(get-file true false false )` protocol frame with extra space between "false" and "false". This is allowed by the protocol spec, but never normally happens, and we do not parse it correctly.
Instead, parse it correctly.
Test Plan:
- Added unit tests.
- Ran `svn proplist svn+ssh://.../diffusion/X/file.c` under SVN 1.10 before and after the change.
- Before: indefinite hang.
- After: completed in finite time.
Reviewers: amckinley, asherkin
Reviewed By: amckinley, asherkin
Maniphest Tasks: T13140
Differential Revision: https://secure.phabricator.com/D19451
+82
-4
+25
-2
src/applications/diffusion/protocol/DiffusionSubversionWireProtocol.php
+25
-2
src/applications/diffusion/protocol/DiffusionSubversionWireProtocol.php
···
33
33
34
34
$messages = array();
35
35
while (true) {
36
-
if ($this->state == 'item') {
36
+
if ($this->state == 'space') {
37
+
// Consume zero or more extra spaces after matching an item. The
38
+
// protocol requires at least one space, but allows more than one.
39
+
40
+
$matches = null;
41
+
if (!preg_match('/^(\s*)\S/', $this->buffer, $matches)) {
42
+
// Wait for more data.
43
+
break;
44
+
}
45
+
46
+
// We have zero or more spaces and then some other character, so throw
47
+
// the spaces away and continue parsing frames.
48
+
if (strlen($matches[1])) {
49
+
$this->buffer = substr($this->buffer, strlen($matches[1]));
50
+
}
51
+
52
+
$this->state = 'item';
53
+
} else if ($this->state == 'item') {
37
54
$match = null;
38
55
$result = null;
39
56
$buf = $this->buffer;
···
69
86
);
70
87
$this->raw = '';
71
88
}
89
+
90
+
// Consume any extra whitespace after an item. If we're in the
91
+
// "bytes" state, we aren't looking for whitespace.
92
+
if ($this->state == 'item') {
93
+
$this->state = 'space';
94
+
}
72
95
} else {
73
96
// No matches yet, wait for more data.
74
97
break;
···
90
113
// Strip off the terminal space.
91
114
$this->pushItem(substr($this->byteBuffer, 0, -1), 'string');
92
115
$this->byteBuffer = '';
93
-
$this->state = 'item';
116
+
$this->state = 'space';
94
117
}
95
118
} else {
96
119
throw new Exception(pht("Invalid state '%s'!", $this->state));
+57
-2
src/applications/diffusion/protocol/__tests__/DiffusionSubversionWireProtocolTestCase.php
+57
-2
src/applications/diffusion/protocol/__tests__/DiffusionSubversionWireProtocolTestCase.php
···
59
59
),
60
60
),
61
61
));
62
+
63
+
// This is testing that multiple spaces are parsed correctly. See T13140
64
+
// for discussion.
65
+
$this->assertSameSubversionMessages(
66
+
'( get-file true false ) ',
67
+
// ^-- Note extra space!
68
+
array(
69
+
array(
70
+
array(
71
+
'type' => 'word',
72
+
'value' => 'get-file',
73
+
),
74
+
array(
75
+
'type' => 'word',
76
+
'value' => 'true',
77
+
),
78
+
array(
79
+
'type' => 'word',
80
+
'value' => 'false',
81
+
),
82
+
),
83
+
),
84
+
'( get-file true false ) ');
85
+
86
+
$this->assertSameSubversionMessages(
87
+
'( duck 5:quack moo ) ',
88
+
array(
89
+
array(
90
+
array(
91
+
'type' => 'word',
92
+
'value' => 'duck',
93
+
),
94
+
array(
95
+
'type' => 'string',
96
+
'value' => 'quack',
97
+
),
98
+
array(
99
+
'type' => 'word',
100
+
'value' => 'moo',
101
+
),
102
+
),
103
+
),
104
+
'( duck 5:quack moo ) ');
105
+
62
106
}
63
107
64
108
public function testSubversionWireProtocolPartialFrame() {
···
86
130
ipull($msg2, 'structure'));
87
131
}
88
132
89
-
private function assertSameSubversionMessages($string, array $structs) {
133
+
private function assertSameSubversionMessages(
134
+
$string,
135
+
array $structs,
136
+
$serial_string = null) {
137
+
90
138
$proto = new DiffusionSubversionWireProtocol();
91
139
92
140
// Verify that the wire message parses into the structs.
···
100
148
$serial[] = $proto->serializeStruct($struct);
101
149
}
102
150
$serial = implode('', $serial);
103
-
$this->assertEqual($string, $serial, 'serialize<'.$string.'>');
151
+
152
+
if ($serial_string === null) {
153
+
$expect_serial = $string;
154
+
} else {
155
+
$expect_serial = $serial_string;
156
+
}
157
+
158
+
$this->assertEqual($expect_serial, $serial, 'serialize<'.$string.'>');
104
159
}
105
160
}