Skip to content

Commit

Permalink
fix(smtp): better HTTP Multipart parsing; fix SMTP body ending detection
Browse files Browse the repository at this point in the history
  • Loading branch information
roxblnfk committed Jun 23, 2024
1 parent da9a205 commit 089e7b5
Show file tree
Hide file tree
Showing 5 changed files with 121 additions and 29 deletions.
33 changes: 15 additions & 18 deletions src/Traffic/Message/Multipart/Part.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,26 +28,23 @@ protected function __construct(
*/
public static function create(array $headers): Part
{
/**
* Check Content-Disposition header
*
* @var string $contentDisposition
*/
$contentDisposition = self::findHeader($headers, 'Content-Disposition')[0]
?? throw new \RuntimeException('Missing Content-Disposition header.');
if ($contentDisposition === '') {
throw new \RuntimeException('Missing Content-Disposition header, can\'t be empty');
}
$contentDisposition = self::findHeader($headers, 'Content-Disposition')[0] ?? null;

$name = $fileName = null;
if ((string) $contentDisposition !== '') {
// Get field name and file name
$name = \preg_match('/\bname=(?:(?<a>[^" ;,]++)|"(?<b>[^"]++)")/', $contentDisposition, $matches) === 1

Check failure on line 36 in src/Traffic/Message/Multipart/Part.php

View workflow job for this annotation

GitHub Actions / psalm (ubuntu-latest, 8.2, locked)

PossiblyNullArgument

src/Traffic/Message/Multipart/Part.php:36:80: PossiblyNullArgument: Argument 2 of preg_match cannot be null, possibly null value provided (see https://psalm.dev/078)

Check failure on line 36 in src/Traffic/Message/Multipart/Part.php

View workflow job for this annotation

GitHub Actions / psalm (ubuntu-latest, 8.2, locked)

PossiblyNullArgument

src/Traffic/Message/Multipart/Part.php:36:80: PossiblyNullArgument: Argument 2 of preg_match cannot be null, possibly null value provided (see https://psalm.dev/078)
? ($matches['a'] ?: $matches['b'])
: null;

// Get field name and file name
$name = \preg_match('/\bname=(?:(?<a>[^" ;,]++)|"(?<b>[^"]++)")/', $contentDisposition, $matches) === 1
? ($matches['a'] ?: $matches['b'])
: null;
// Decode file name
$fileName = \preg_match(
'/\bfilename=(?:(?<a>[^" ;,]++)|"(?<b>[^"]++)")/',
$contentDisposition,

Check failure on line 43 in src/Traffic/Message/Multipart/Part.php

View workflow job for this annotation

GitHub Actions / psalm (ubuntu-latest, 8.2, locked)

PossiblyNullArgument

src/Traffic/Message/Multipart/Part.php:43:17: PossiblyNullArgument: Argument 2 of preg_match cannot be null, possibly null value provided (see https://psalm.dev/078)

Check failure on line 43 in src/Traffic/Message/Multipart/Part.php

View workflow job for this annotation

GitHub Actions / psalm (ubuntu-latest, 8.2, locked)

PossiblyNullArgument

src/Traffic/Message/Multipart/Part.php:43:17: PossiblyNullArgument: Argument 2 of preg_match cannot be null, possibly null value provided (see https://psalm.dev/078)
$matches,
) === 1 ? ($matches['a'] ?: $matches['b']) : null;
}

// Decode file name
$fileName = \preg_match('/\bfilename=(?:(?<a>[^" ;,]++)|"(?<b>[^"]++)")/', $contentDisposition, $matches) === 1
? ($matches['a'] ?: $matches['b'])
: null;
$fileName = $fileName !== null ? \html_entity_decode($fileName) : null;
$isFile = (string) $fileName !== ''
|| \preg_match('/text\\/.++/', self::findHeader($headers, 'Content-Type')[0] ?? 'text/plain') !== 1;
Expand Down
2 changes: 1 addition & 1 deletion src/Traffic/Parser/Http.php
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ private function parseBody(StreamClient $stream, ServerRequestInterface $request
$contentType = $request->getHeaderLine('Content-Type');
return match (true) {
$contentType === 'application/x-www-form-urlencoded' => self::parseUrlEncodedBody($request),
\str_contains($contentType, 'multipart/form-data') => $this->processMultipartForm($request),
\str_contains($contentType, 'multipart/') => $this->processMultipartForm($request),
default => $request,
};
}
Expand Down
39 changes: 39 additions & 0 deletions src/Traffic/Parser/MultipartType.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
<?php

declare(strict_types=1);

namespace Buggregator\Trap\Traffic\Parser;

/**
* @internal
*/
enum MultipartType: string
{
case Mixed = 'mixed';
case Alternative = 'alternative';
case Digest = 'digest';
case Parallel = 'parallel';
case FormData = 'form-data';
case Report = 'report';
case Signed = 'signed';
case Encrypted = 'encrypted';
case Related = 'related';
case Other = 'other';

public static function fromHeaderLine(string $headerLine): self

Check warning on line 23 in src/Traffic/Parser/MultipartType.php

View check run for this annotation

Codecov / codecov/patch

src/Traffic/Parser/MultipartType.php#L23

Added line #L23 was not covered by tests
{
$type = \explode(';', $headerLine, 2)[0];
return match ($type) {
'multipart/mixed' => self::Mixed,
'multipart/alternative' => self::Alternative,
'multipart/digest' => self::Digest,
'multipart/parallel' => self::Parallel,
'multipart/form-data' => self::FormData,
'multipart/report' => self::Report,
'multipart/signed' => self::Signed,
'multipart/encrypted' => self::Encrypted,
'multipart/related' => self::Related,
default => self::Other,
};

Check warning on line 37 in src/Traffic/Parser/MultipartType.php

View check run for this annotation

Codecov / codecov/patch

src/Traffic/Parser/MultipartType.php#L25-L37

Added lines #L25 - L37 were not covered by tests
}
}
23 changes: 15 additions & 8 deletions src/Traffic/Parser/Smtp.php
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ public function parseStream(array $protocol, StreamClient $stream): Message\Smtp
$message = Message\Smtp::create($protocol, headers: $headers);

// Defaults
$boundary = "\r\n.\r\n";
$endOfStream = ["\r\n.\r\n"];
$isMultipart = false;

// Check the message is multipart.
Expand All @@ -41,10 +41,11 @@ public function parseStream(array $protocol, StreamClient $stream): Message\Smtp
&& \preg_match('/boundary="?([^"\\s;]++)"?/', $contentType, $matches) === 1
) {
$isMultipart = true;
$boundary = "\r\n--{$matches[1]}--\r\n\r\n";
$endOfStream = ["\r\n--{$matches[1]}--\r\n\r\n"];
$endOfStream[] = $endOfStream[0] . ".\r\n";
}

$stored = $this->storeBody($fileStream, $stream, $boundary);
$stored = $this->storeBody($fileStream, $stream, $endOfStream);
$message = $message->withBody($fileStream);
// Message's body must be seeked to the beginning of the body.
$fileStream->seek(-$stored, \SEEK_CUR);
Expand All @@ -62,16 +63,19 @@ public function parseStream(array $protocol, StreamClient $stream): Message\Smtp
* Flush stream data into PSR stream.
* Note: there can be read more data than {@see $limit} bytes but write only {@see $limit} bytes.
*
* @return int Number of bytes written to the stream.
* @param non-empty-array<non-empty-string> $endings
*
* @return int<0, max> Number of bytes written to the stream.
*/
private function storeBody(
StreamInterface $fileStream,
StreamClient $stream,
string $end = "\r\n.\r\n",
array $endings = ["\r\n.\r\n"],
): int {
$written = 0;
$endLen = \strlen($end);
$endLen = \min(\array_map('\strlen', $endings));

/** @var string $chunk */
foreach ($stream->getIterator() as $chunk) {
// Write chunk to the file stream.
$fileStream->write($chunk);
Expand All @@ -83,8 +87,11 @@ private function storeBody(
$fileStream->seek(-$endLen, \SEEK_CUR);
$chunk = $fileStream->read($endLen);
}
if (\str_ends_with($chunk, $end)) {
return $written;

foreach ($endings as $end) {
if (\str_ends_with($chunk, $end)) {
return $written;
}
}
}

Expand Down
53 changes: 51 additions & 2 deletions tests/Unit/Traffic/Parser/MultipartBodyParserTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
use Buggregator\Trap\Traffic\Message\Multipart\File;
use Buggregator\Trap\Traffic\Message\Multipart\Part;
use Buggregator\Trap\Traffic\Parser;
use Buggregator\Trap\Traffic\Parser\MultipartType;
use Nyholm\Psr7\Stream;
use PHPUnit\Framework\TestCase;
use Psr\Http\Message\StreamInterface;
Expand Down Expand Up @@ -159,6 +160,51 @@ public function testBase64Encoded(): void
self::assertSame($file2, $file->getStream()->__toString());
}

/**
* Simple multipart/mixed message without nested parts.
*/
public function testMultipartMixed(): void
{
$body = $this->makeStream(
<<<BODY
--40ugHb8e\r
Content-Type: text/plain; charset=utf-8\r
Content-Transfer-Encoding: quoted-printable\r
\r
Test Body\r
--40ugHb8e\r
Content-Type: text/plain; name=test.txt\r
Content-Transfer-Encoding: base64\r
Content-Disposition: attachment; name=test.txt; filename=test.txt\r
\r
c2RnCg==\r
--40ugHb8e--\r
\r
.\r
BODY,
);

$result = $this->parse($body, '40ugHb8e', MultipartType::Mixed);


self::assertCount(2, $result);
// Field
$file = $result[0];
self::assertInstanceOf(Field::class, $file);
self::assertNull($file->getName());
self::assertSame('Test Body', $file->getValue());

// Attached file
$file = $result[1];
self::assertInstanceOf(File::class, $file);
self::assertSame('test.txt', $file->getName());
self::assertSame('test.txt', $file->getClientFilename());
self::assertSame('text/plain', $file->getClientMediaType());
self::assertNull($file->getEmbeddingId());
self::assertSame("sdg\n", $file->getStream()->__toString());
}

private function makeStream(string $body): StreamInterface
{
$stream = Stream::create($body);
Expand All @@ -171,8 +217,11 @@ private function makeStream(string $body): StreamInterface
*
* @return iterable<Part>
*/
private function parse(StreamInterface $body, string $boundary): iterable
{
private function parse(
StreamInterface $body,
string $boundary,
MultipartType $type = MultipartType::FormData,
): iterable {
return $this->runInFiber(static fn() => Parser\Http::parseMultipartBody($body, $boundary));
}
}

0 comments on commit 089e7b5

Please sign in to comment.