Skip to content

Commit e79eea1

Browse files
nicolas-grekasnicolas-grekas
committed
bug #57453 [HttpClient] Fix parsing SSE (fancyweb)
This PR was merged into the 5.4 branch. Discussion ---------- [HttpClient] Fix parsing SSE | Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Issues | - | License | MIT This PR fixes at least 2 behaviors: * Correctly parse comments to ignore them (yield them as DataChunk) * Avoid yielding a final empty SSE I also reworked the test to leverage MockHttpClient. Commits ------- add5d45 [HttpClient] Fix parsing SSE
2 parents 7085623 + add5d45 commit e79eea1

File tree

2 files changed

+55
-60
lines changed

2 files changed

+55
-60
lines changed

‎src/Symfony/Component/HttpClient/EventSourceHttpClient.php

+20-17
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
namespace Symfony\Component\HttpClient;
1313

14+
use Symfony\Component\HttpClient\Chunk\DataChunk;
1415
use Symfony\Component\HttpClient\Chunk\ServerSentEvent;
1516
use Symfony\Component\HttpClient\Exception\EventSourceException;
1617
use Symfony\Component\HttpClient\Response\AsyncContext;
@@ -121,17 +122,30 @@ public function request(string $method, string $url, array $options = []): Respo
121122
return;
122123
}
123124

124-
$rx = '/((?:\r\n){2,}|\r{2,}|\n{2,})/';
125-
$content = $state->buffer.$chunk->getContent();
126-
127125
if ($chunk->isLast()) {
128-
$rx = substr_replace($rx, '|$', -2, 0);
126+
if ('' !== $content = $state->buffer) {
127+
$state->buffer = '';
128+
yield new DataChunk(-1, $content);
129+
}
130+
131+
yield $chunk;
132+
133+
return;
129134
}
130-
$events = preg_split($rx, $content, -1, \PREG_SPLIT_DELIM_CAPTURE);
135+
136+
$content = $state->buffer.$chunk->getContent();
137+
$events = preg_split('/((?:\r\n){2,}|\r{2,}|\n{2,})/', $content, -1, \PREG_SPLIT_DELIM_CAPTURE);
131138
$state->buffer = array_pop($events);
132139

133140
for ($i = 0; isset($events[$i]); $i += 2) {
134-
$event = new ServerSentEvent($events[$i].$events[1 + $i]);
141+
$content = $events[$i].$events[1 + $i];
142+
if (!preg_match('/(?:^|\r\n|[\r\n])[^:\r\n]/', $content)) {
143+
yield new DataChunk(-1, $content);
144+
145+
continue;
146+
}
147+
148+
$event = new ServerSentEvent($content);
135149

136150
if ('' !== $event->getId()) {
137151
$context->setInfo('last_event_id', $state->lastEventId = $event->getId());
@@ -143,17 +157,6 @@ public function request(string $method, string $url, array $options = []): Respo
143157

144158
yield $event;
145159
}
146-
147-
if (preg_match('/^(?::[^\r\n]*+(?:\r\n|[\r\n]))+$/m', $state->buffer)) {
148-
$content = $state->buffer;
149-
$state->buffer = '';
150-
151-
yield $context->createChunk($content);
152-
}
153-
154-
if ($chunk->isLast()) {
155-
yield $chunk;
156-
}
157160
});
158161
}
159162
}

‎src/Symfony/Component/HttpClient/Tests/EventSourceHttpClientTest.php

+35-43
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,11 @@
1515
use Symfony\Component\HttpClient\Chunk\DataChunk;
1616
use Symfony\Component\HttpClient\Chunk\ErrorChunk;
1717
use Symfony\Component\HttpClient\Chunk\FirstChunk;
18+
use Symfony\Component\HttpClient\Chunk\LastChunk;
1819
use Symfony\Component\HttpClient\Chunk\ServerSentEvent;
1920
use Symfony\Component\HttpClient\EventSourceHttpClient;
2021
use Symfony\Component\HttpClient\Exception\EventSourceException;
22+
use Symfony\Component\HttpClient\MockHttpClient;
2123
use Symfony\Component\HttpClient\Response\MockResponse;
2224
use Symfony\Component\HttpClient\Response\ResponseStream;
2325
use Symfony\Contracts\HttpClient\HttpClientInterface;
@@ -34,7 +36,11 @@ class EventSourceHttpClientTest extends TestCase
3436
*/
3537
public function testGetServerSentEvents(string $sep)
3638
{
37-
$rawData = <<<TXT
39+
$es = new EventSourceHttpClient(new MockHttpClient(function (string $method, string $url, array $options) use ($sep): MockResponse {
40+
$this->assertSame(['Accept: text/event-stream', 'Cache-Control: no-cache'], $options['headers']);
41+
42+
return new MockResponse([
43+
str_replace("\n", $sep, <<<TXT
3844
event: builderror
3945
id: 46
4046
data: {"foo": "bar"}
@@ -43,7 +49,18 @@ public function testGetServerSentEvents(string $sep)
4349
id: 47
4450
data: {}
4551
52+
: this is a oneline comment
53+
54+
: this is a
55+
: multiline comment
56+
57+
: comments are ignored
4658
event: reload
59+
60+
TXT
61+
),
62+
str_replace("\n", $sep, <<<TXT
63+
: anywhere
4764
id: 48
4865
data: {}
4966
@@ -62,58 +79,33 @@ public function testGetServerSentEvents(string $sep)
6279
6380
id: 60
6481
data
65-
TXT;
66-
$data = str_replace("\n", $sep, $rawData);
67-
68-
$chunk = new DataChunk(0, $data);
69-
$response = new MockResponse('', ['canceled' => false, 'http_method' => 'GET', 'url' => 'http://localhost:8080/events', 'response_headers' => ['content-type: text/event-stream']]);
70-
$responseStream = new ResponseStream((function () use ($response, $chunk) {
71-
yield $response => new FirstChunk();
72-
yield $response => $chunk;
73-
yield $response => new ErrorChunk(0, 'timeout');
74-
})());
75-
76-
$hasCorrectHeaders = function ($options) {
77-
$this->assertSame(['Accept: text/event-stream', 'Cache-Control: no-cache'], $options['headers']);
78-
79-
return true;
80-
};
81-
82-
$httpClient = $this->createMock(HttpClientInterface::class);
83-
$httpClient->method('request')->with('GET', 'http://localhost:8080/events', $this->callback($hasCorrectHeaders))->willReturn($response);
84-
85-
$httpClient->method('stream')->willReturn($responseStream);
86-
87-
$es = new EventSourceHttpClient($httpClient);
82+
TXT
83+
),
84+
], [
85+
'canceled' => false,
86+
'http_method' => 'GET',
87+
'url' => 'http://localhost:8080/events',
88+
'response_headers' => ['content-type: text/event-stream'],
89+
]);
90+
}));
8891
$res = $es->connect('http://localhost:8080/events');
8992

9093
$expected = [
9194
new FirstChunk(),
9295
new ServerSentEvent(str_replace("\n", $sep, "event: builderror\nid: 46\ndata: {\"foo\": \"bar\"}\n\n")),
9396
new ServerSentEvent(str_replace("\n", $sep, "event: reload\nid: 47\ndata: {}\n\n")),
94-
new ServerSentEvent(str_replace("\n", $sep, "event: reload\nid: 48\ndata: {}\n\n")),
97+
new DataChunk(-1, str_replace("\n", $sep, ": this is a oneline comment\n\n")),
98+
new DataChunk(-1, str_replace("\n", $sep, ": this is a\n: multiline comment\n\n")),
99+
new ServerSentEvent(str_replace("\n", $sep, ": comments are ignored\nevent: reload\n: anywhere\nid: 48\ndata: {}\n\n")),
95100
new ServerSentEvent(str_replace("\n", $sep, "data: test\ndata:test\nid: 49\nevent: testEvent\n\n\n")),
96101
new ServerSentEvent(str_replace("\n", $sep, "id: 50\ndata: <tag>\ndata\ndata: <foo />\ndata\ndata: </tag>\n\n")),
102+
new DataChunk(-1, str_replace("\n", $sep, "id: 60\ndata")),
103+
new LastChunk("\r\n" === $sep ? 355 : 322),
97104
];
98-
$i = 0;
99-
100-
$this->expectExceptionMessage('Response has been canceled');
101-
while ($res) {
102-
if ($i > 0) {
103-
$res->cancel();
104-
}
105-
foreach ($es->stream($res) as $chunk) {
106-
if ($chunk->isTimeout()) {
107-
continue;
108-
}
109-
110-
if ($chunk->isLast()) {
111-
continue;
112-
}
113-
114-
$this->assertEquals($expected[$i++], $chunk);
115-
}
105+
foreach ($es->stream($res) as $chunk) {
106+
$this->assertEquals(array_shift($expected), $chunk);
116107
}
108+
$this->assertSame([], $expected);
117109
}
118110

119111
/**

0 commit comments

Comments
 (0)