Skip to content

Commit 308093c

Browse files
authored
Merge pull request #121 from DirectoryTree/bug-115
Fix issue with multi-line IMAP response parsing
2 parents a9420fc + 11547f7 commit 308093c

File tree

4 files changed

+183
-53
lines changed

4 files changed

+183
-53
lines changed

.github/workflows/run-integration-tests.yml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ name: run-integration-tests
22

33
on:
44
push:
5+
branches:
6+
- master
57
pull_request:
68
schedule:
79
- cron: "0 0 * * *"

.github/workflows/run-tests.yml

Lines changed: 39 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1,42 +1,44 @@
11
name: run-tests
22

33
on:
4-
push:
5-
pull_request:
6-
schedule:
7-
- cron: "0 0 * * *"
4+
push:
5+
branches:
6+
- master
7+
pull_request:
8+
schedule:
9+
- cron: "0 0 * * *"
810

911
jobs:
10-
run-tests:
11-
runs-on: ${{ matrix.os }}
12-
13-
strategy:
14-
fail-fast: false
15-
matrix:
16-
os: [ ubuntu-latest ]
17-
php: [ 8.4, 8.3, 8.2, 8.1 ]
18-
dependency-version: [ prefer-stable ]
19-
20-
name: ${{ matrix.os }} - P${{ matrix.php }} - ${{ matrix.dependency-version }}
21-
22-
steps:
23-
- name: Checkout code
24-
uses: actions/checkout@v4
25-
26-
- name: Cache dependencies
27-
uses: actions/cache@v4
28-
with:
29-
path: ~/.composer/cache/files
30-
key: dependencies-laravel-php-${{ matrix.php }}-composer-${{ hashFiles('composer.json') }}
31-
32-
- name: Setup PHP
33-
uses: shivammathur/setup-php@v2
34-
with:
35-
php-version: ${{ matrix.php }}
36-
37-
- name: Install dependencies
38-
run: |
39-
composer update --${{ matrix.dependency-version }} --prefer-dist --no-interaction
40-
41-
- name: Execute tests
42-
run: vendor/bin/pest --testsuite Unit
12+
run-tests:
13+
runs-on: ${{ matrix.os }}
14+
15+
strategy:
16+
fail-fast: false
17+
matrix:
18+
os: [ ubuntu-latest ]
19+
php: [ 8.4, 8.3, 8.2, 8.1 ]
20+
dependency-version: [ prefer-stable ]
21+
22+
name: ${{ matrix.os }} - P${{ matrix.php }} - ${{ matrix.dependency-version }}
23+
24+
steps:
25+
- name: Checkout code
26+
uses: actions/checkout@v4
27+
28+
- name: Cache dependencies
29+
uses: actions/cache@v4
30+
with:
31+
path: ~/.composer/cache/files
32+
key: dependencies-laravel-php-${{ matrix.php }}-composer-${{ hashFiles('composer.json') }}
33+
34+
- name: Setup PHP
35+
uses: shivammathur/setup-php@v2
36+
with:
37+
php-version: ${{ matrix.php }}
38+
39+
- name: Install dependencies
40+
run: |
41+
composer update --${{ matrix.dependency-version }} --prefer-dist --no-interaction
42+
43+
- name: Execute tests
44+
run: vendor/bin/pest --testsuite Unit

src/Connection/ImapParser.php

Lines changed: 20 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -88,12 +88,12 @@ protected function parseUntaggedResponse(): UntaggedResponse
8888
$this->advance();
8989

9090
// Collect all tokens until the end-of-response marker.
91-
while ($this->currentToken && ! $this->isEndOfResponseToken($this->currentToken)) {
91+
while ($this->currentToken && ! $this->currentToken instanceof Crlf) {
9292
$elements[] = $this->parseElement();
9393
}
9494

9595
// If the end-of-response marker (CRLF) is present, consume it.
96-
if ($this->currentToken && $this->isEndOfResponseToken($this->currentToken)) {
96+
if ($this->currentToken && $this->currentToken instanceof Crlf) {
9797
$this->currentToken = null;
9898
} else {
9999
throw new ImapParserException('Unterminated untagged response');
@@ -116,12 +116,12 @@ protected function parseContinuationResponse(): ContinuationResponse
116116
$this->advance();
117117

118118
// Collect all tokens until the CRLF marker.
119-
while ($this->currentToken && ! $this->isEndOfResponseToken($this->currentToken)) {
119+
while ($this->currentToken && ! $this->currentToken instanceof Crlf) {
120120
$elements[] = $this->parseElement();
121121
}
122122

123123
// Consume the CRLF marker if present.
124-
if ($this->currentToken && $this->isEndOfResponseToken($this->currentToken)) {
124+
if ($this->currentToken && $this->currentToken instanceof Crlf) {
125125
$this->currentToken = null;
126126
} else {
127127
throw new ImapParserException('Unterminated continuation response');
@@ -144,12 +144,12 @@ protected function parseTaggedResponse(): TaggedResponse
144144
$this->advance();
145145

146146
// Collect tokens until the end-of-response marker is reached.
147-
while ($this->currentToken && ! $this->isEndOfResponseToken($this->currentToken)) {
147+
while ($this->currentToken && ! $this->currentToken instanceof Crlf) {
148148
$tokens[] = $this->parseElement();
149149
}
150150

151151
// Consume the CRLF marker if present.
152-
if ($this->currentToken && $this->isEndOfResponseToken($this->currentToken)) {
152+
if ($this->currentToken && $this->currentToken instanceof Crlf) {
153153
$this->currentToken = null;
154154
} else {
155155
throw new ImapParserException('Unterminated tagged response');
@@ -173,8 +173,14 @@ protected function parseBracketGroup(): ResponseCodeData
173173
while (
174174
$this->currentToken
175175
&& ! $this->currentToken instanceof ResponseCodeClose
176-
&& ! $this->isEndOfResponseToken($this->currentToken)
177176
) {
177+
// Skip CRLF tokens that may appear inside bracket groups.
178+
if ($this->currentToken instanceof Crlf) {
179+
$this->advance();
180+
181+
continue;
182+
}
183+
178184
$elements[] = $this->parseElement();
179185
}
180186

@@ -204,8 +210,14 @@ protected function parseList(): ListData
204210
while (
205211
$this->currentToken
206212
&& ! $this->currentToken instanceof ListClose
207-
&& ! $this->isEndOfResponseToken($this->currentToken)
208213
) {
214+
// Skip CRLF tokens that appear inside lists (after literals).
215+
if ($this->currentToken instanceof Crlf) {
216+
$this->advance();
217+
218+
continue;
219+
}
220+
209221
$elements[] = $this->parseElement();
210222
}
211223

@@ -255,12 +267,4 @@ protected function advance(): void
255267
{
256268
$this->currentToken = $this->tokenizer->nextToken();
257269
}
258-
259-
/**
260-
* Determine if the given token marks the end of a response.
261-
*/
262-
protected function isEndOfResponseToken(Token $token): bool
263-
{
264-
return $token instanceof Crlf;
265-
}
266270
}

tests/Unit/Connection/ImapParserTest.php

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -267,3 +267,125 @@
267267
"* 1 FETCH (BODY {14}\r\nHello World!\r\n)",
268268
],
269269
]);
270+
271+
test('parses fetch response with body text then header', function () {
272+
$stream = new FakeStream;
273+
$stream->open();
274+
275+
// Simulating BODY[TEXT] before BODY[HEADER]
276+
$stream->feedRaw([
277+
"* 1 FETCH (UID 123 FLAGS (\\Seen) BODY[TEXT] {13}\r\n",
278+
"Hello World\r\n",
279+
" BODY[HEADER] {23}\r\n",
280+
"Subject: Test Message\r\n",
281+
")\r\n",
282+
]);
283+
284+
$tokenizer = new ImapTokenizer($stream);
285+
$parser = new ImapParser($tokenizer);
286+
287+
$response = $parser->next();
288+
289+
expect($response)->toBeInstanceOf(UntaggedResponse::class);
290+
291+
// Get the ListData at index 3 (the FETCH data)
292+
$data = $response->tokenAt(3);
293+
expect($data)->toBeInstanceOf(ListData::class);
294+
295+
// Verify we can lookup UID
296+
$uid = $data->lookup('UID');
297+
expect($uid)->not->toBeNull();
298+
expect($uid->value)->toBe('123');
299+
300+
// Verify we can lookup FLAGS
301+
$flags = $data->lookup('FLAGS');
302+
expect($flags)->not->toBeNull();
303+
304+
// Verify we can lookup both BODY sections with correct content
305+
$text = $data->lookup('[TEXT]');
306+
expect($text)->not->toBeNull();
307+
expect($text->value)->toBe("Hello World\r\n");
308+
309+
$header = $data->lookup('[HEADER]');
310+
expect($header)->not->toBeNull();
311+
expect($header->value)->toBe("Subject: Test Message\r\n");
312+
});
313+
314+
test('parses fetch response with body header then text', function () {
315+
$stream = new FakeStream;
316+
$stream->open();
317+
318+
// Simulating BODY[HEADER] before BODY[TEXT]
319+
$stream->feedRaw([
320+
"* 1 FETCH (UID 456 FLAGS (\\Seen) BODY[HEADER] {26}\r\n",
321+
"From: [email protected]\r\n",
322+
" BODY[TEXT] {20}\r\n",
323+
"Message body here.\r\n",
324+
")\r\n",
325+
]);
326+
327+
$tokenizer = new ImapTokenizer($stream);
328+
$parser = new ImapParser($tokenizer);
329+
330+
$response = $parser->next();
331+
332+
expect($response)->toBeInstanceOf(UntaggedResponse::class);
333+
334+
// Get the ListData at index 3 (the FETCH data)
335+
$data = $response->tokenAt(3);
336+
expect($data)->toBeInstanceOf(ListData::class);
337+
338+
// Verify we can lookup UID
339+
$uid = $data->lookup('UID');
340+
expect($uid)->not->toBeNull();
341+
expect($uid->value)->toBe('456');
342+
343+
// Verify we can lookup FLAGS
344+
$flags = $data->lookup('FLAGS');
345+
expect($flags)->not->toBeNull();
346+
expect($flags->tokens())->toHaveCount(1);
347+
expect($flags->tokenAt(0)->value)->toBe('\\Seen');
348+
349+
// Verify we can lookup both BODY sections with correct content
350+
$header = $data->lookup('[HEADER]');
351+
expect($header)->not->toBeNull();
352+
expect($header->value)->toBe("From: [email protected]\r\n");
353+
354+
$text = $data->lookup('[TEXT]');
355+
expect($text)->not->toBeNull();
356+
expect($text->value)->toBe("Message body here.\r\n");
357+
});
358+
359+
test('parses fetch response with all metadata and body parts', function () {
360+
$stream = new FakeStream;
361+
$stream->open();
362+
363+
// Full FETCH response with all common fields
364+
$stream->feedRaw([
365+
"* 1 FETCH (UID 789 RFC822.SIZE 1024 FLAGS (\\Seen \\Flagged) BODY[TEXT] {25}\r\n",
366+
"This is the email body.\r\n",
367+
" BODY[HEADER] {46}\r\n",
368+
"To: [email protected]\r\nSubject: Re: Test\r\n",
369+
")\r\n",
370+
]);
371+
372+
$tokenizer = new ImapTokenizer($stream);
373+
$parser = new ImapParser($tokenizer);
374+
375+
$response = $parser->next();
376+
377+
expect($response)->toBeInstanceOf(UntaggedResponse::class);
378+
379+
$data = $response->tokenAt(3);
380+
expect($data)->toBeInstanceOf(ListData::class);
381+
382+
$flags = $data->lookup('FLAGS')->tokens();
383+
384+
expect($flags)->toHaveCount(2);
385+
expect($flags[0]->value)->toBe('\\Seen');
386+
expect($flags[1]->value)->toBe('\\Flagged');
387+
expect($data->lookup('UID')?->value)->toBe('789');
388+
expect($data->lookup('RFC822.SIZE')?->value)->toBe('1024');
389+
expect($data->lookup('[TEXT]')->value)->toBe("This is the email body.\r\n");
390+
expect($data->lookup('[HEADER]')->value)->toBe("To: [email protected]\r\nSubject: Re: Test\r\n");
391+
});

0 commit comments

Comments
 (0)