From 283740b728218bd02974d2d8b4012f8a6941529c Mon Sep 17 00:00:00 2001 From: Aleksei Gagarin Date: Sun, 23 Jun 2024 18:22:02 +0400 Subject: [PATCH 01/21] fix(smtp): fix SMTP body ending detection; better HTTP Multipart parsing --- README.md | 10 ++-- src/Traffic/Message/Multipart/Part.php | 33 ++++++------ src/Traffic/Parser/Http.php | 2 +- src/Traffic/Parser/MultipartType.php | 39 ++++++++++++++ src/Traffic/Parser/Smtp.php | 23 +++++--- .../Parser/MultipartBodyParserTest.php | 53 ++++++++++++++++++- 6 files changed, 126 insertions(+), 34 deletions(-) create mode 100644 src/Traffic/Parser/MultipartType.php diff --git a/README.md b/README.md index ad8c724a..64cdaed4 100644 --- a/README.md +++ b/README.md @@ -27,11 +27,11 @@ Trap includes: **Table of content:** -* [Installation](#installation) -* [Overview](#overview) -* [Usage](#usage) -* [Contributing](#contributing) -* [License](#license) +- [Installation](#installation) +- [Overview](#overview) +- [Usage](#usage) +- [Contributing](#contributing) +- [License](#license) ## Installation diff --git a/src/Traffic/Message/Multipart/Part.php b/src/Traffic/Message/Multipart/Part.php index 8c93408b..1a6c64f8 100644 --- a/src/Traffic/Message/Multipart/Part.php +++ b/src/Traffic/Message/Multipart/Part.php @@ -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=(?:(?[^" ;,]++)|"(?[^"]++)")/', $contentDisposition, $matches) === 1 + ? ($matches['a'] ?: $matches['b']) + : null; - // Get field name and file name - $name = \preg_match('/\bname=(?:(?[^" ;,]++)|"(?[^"]++)")/', $contentDisposition, $matches) === 1 - ? ($matches['a'] ?: $matches['b']) - : null; + // Decode file name + $fileName = \preg_match( + '/\bfilename=(?:(?[^" ;,]++)|"(?[^"]++)")/', + $contentDisposition, + $matches, + ) === 1 ? ($matches['a'] ?: $matches['b']) : null; + } - // Decode file name - $fileName = \preg_match('/\bfilename=(?:(?[^" ;,]++)|"(?[^"]++)")/', $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; diff --git a/src/Traffic/Parser/Http.php b/src/Traffic/Parser/Http.php index 85ccee34..3441895e 100644 --- a/src/Traffic/Parser/Http.php +++ b/src/Traffic/Parser/Http.php @@ -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, }; } diff --git a/src/Traffic/Parser/MultipartType.php b/src/Traffic/Parser/MultipartType.php new file mode 100644 index 00000000..5215734b --- /dev/null +++ b/src/Traffic/Parser/MultipartType.php @@ -0,0 +1,39 @@ + 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, + }; + } +} diff --git a/src/Traffic/Parser/Smtp.php b/src/Traffic/Parser/Smtp.php index 50310489..2cfd6e84 100644 --- a/src/Traffic/Parser/Smtp.php +++ b/src/Traffic/Parser/Smtp.php @@ -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. @@ -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); @@ -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 $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); @@ -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; + } } } diff --git a/tests/Unit/Traffic/Parser/MultipartBodyParserTest.php b/tests/Unit/Traffic/Parser/MultipartBodyParserTest.php index 2f4b303c..f7ac0c6f 100644 --- a/tests/Unit/Traffic/Parser/MultipartBodyParserTest.php +++ b/tests/Unit/Traffic/Parser/MultipartBodyParserTest.php @@ -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; @@ -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( + <<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); @@ -171,8 +217,11 @@ private function makeStream(string $body): StreamInterface * * @return iterable */ - 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)); } } From 156ac1e0386a40454e71f1282f3b10bed6e34a12 Mon Sep 17 00:00:00 2001 From: Aleksei Gagarin Date: Sun, 23 Jun 2024 18:24:42 +0400 Subject: [PATCH 02/21] chore(master): release 1.10.1 (#125) --- CHANGELOG.md | 8 ++++++++ resources/version.json | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index da47a11f..850146f6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,13 @@ # Changelog +## 1.10.1 (2024-06-23) + +## What's Changed +* Better HTTP/SMTP Multipart parsing by @roxblnfk in https://github.com/buggregator/trap/pull/128 + + +**Full Changelog**: https://github.com/buggregator/trap/compare/1.10.0...1.10.1 + ## 1.10.0 (2024-06-20) ## What's Changed diff --git a/resources/version.json b/resources/version.json index ce4f35c2..5009cc6b 100644 --- a/resources/version.json +++ b/resources/version.json @@ -1,3 +1,3 @@ { - ".": "1.10.0" + ".": "1.10.1" } From d5e167519f7f887a4224c9a9ed3ebcb14542ee69 Mon Sep 17 00:00:00 2001 From: Plakhotnikov Vladimir Date: Sat, 29 Jun 2024 21:10:57 +0300 Subject: [PATCH 03/21] style: fix Psalm issues --- psalm-baseline.xml | 38 +------------------ src/Client/Caster/ProtobufCaster.php | 6 ++- src/Sender/Console/Renderer/Profiler.php | 6 +-- .../Console/Renderer/Sentry/Exceptions.php | 11 +++--- src/Sender/Console/Renderer/VarDumper.php | 16 +++++--- src/Sender/Console/Support/Tables.php | 9 +++-- src/Support/Stream/Base64DecodeFilter.php | 2 +- src/Traffic/Message/Multipart/Part.php | 5 ++- src/Traffic/Parser/Http.php | 5 ++- 9 files changed, 39 insertions(+), 59 deletions(-) diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 438f66cf..d619aa13 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -92,11 +92,6 @@ - - - - - getCookieParams()]]> @@ -113,11 +108,8 @@ - - - $output->writeln(]]> - + @@ -129,11 +121,6 @@ - - - - - @@ -161,39 +148,19 @@ - - - - - - - - - - - - - - - - - - - - @@ -218,8 +185,5 @@ getName()]]]> - - - diff --git a/src/Client/Caster/ProtobufCaster.php b/src/Client/Caster/ProtobufCaster.php index 05dd8f4b..58393b14 100644 --- a/src/Client/Caster/ProtobufCaster.php +++ b/src/Client/Caster/ProtobufCaster.php @@ -55,8 +55,12 @@ final class ProtobufCaster public static function cast(Message $c, array $a, Stub $stub, bool $isNested): array { + /** + * @var DescriptorPool $pool + */ + $pool = DescriptorPool::getGeneratedPool(); /** @var PublicDescriptor|InternalDescriptor $descriptor */ - $descriptor = DescriptorPool::getGeneratedPool()->getDescriptorByClassName($c::class); + $descriptor = $pool->getDescriptorByClassName($c::class); return self::castMessage($c, $descriptor); } diff --git a/src/Sender/Console/Renderer/Profiler.php b/src/Sender/Console/Renderer/Profiler.php index 7d7cdaf9..866cdc2b 100644 --- a/src/Sender/Console/Renderer/Profiler.php +++ b/src/Sender/Console/Renderer/Profiler.php @@ -61,9 +61,9 @@ public function render(OutputInterface $output, Frame $frame): void Tables::renderKeyValueTable($output, 'Peak values', [ 'Memory usage' => Measure::memory($peaks->mu), 'Peak memory usage' => Measure::memory($peaks->pmu), - 'Wall time' => $peaks->wt, - 'CPU time' => $peaks->cpu, - 'Calls count' => $peaks->ct, + 'Wall time' => (string) $peaks->wt, + 'CPU time' => (string) $peaks->cpu, + 'Calls count' => (string) $peaks->ct, ]); } } diff --git a/src/Sender/Console/Renderer/Sentry/Exceptions.php b/src/Sender/Console/Renderer/Sentry/Exceptions.php index d8fffa7c..138acfcb 100644 --- a/src/Sender/Console/Renderer/Sentry/Exceptions.php +++ b/src/Sender/Console/Renderer/Sentry/Exceptions.php @@ -64,7 +64,8 @@ private static function renderTrace(OutputInterface $output, array $frames, bool if ($frames === []) { return; } - $getValue = static fn(array $frame, string $key, ?string $default = ''): string|int|float|bool|null => + + $getValue = static fn(array $frame, string $key, string $default = ''): string|int|float|bool => isset($frame[$key]) && \is_scalar($frame[$key]) ? $frame[$key] : $default; $i = \count($frames) ; @@ -80,7 +81,7 @@ private static function renderTrace(OutputInterface $output, array $frames, bool } $file = $getValue($frame, 'filename'); - $line = $getValue($frame, 'lineno', null); + $line = $getValue($frame, 'lineno'); $class = $getValue($frame, 'class'); /** @psalm-suppress RiskyTruthyFalsyComparison */ $class = empty($class) ? '' : $class . '::'; @@ -90,11 +91,11 @@ private static function renderTrace(OutputInterface $output, array $frames, bool \sprintf( "%s%s%s\n%s%s%s()", \str_pad("#$i", $numPad, ' '), - $file, - !$line ? '' : ":$line", + (string) $file, + $line !== '' ? ":$line": '', \str_repeat(' ', $numPad), $class, - $function, + (string) $function, ), ); diff --git a/src/Sender/Console/Renderer/VarDumper.php b/src/Sender/Console/Renderer/VarDumper.php index 88c80700..7bc3f390 100644 --- a/src/Sender/Console/Renderer/VarDumper.php +++ b/src/Sender/Console/Renderer/VarDumper.php @@ -23,6 +23,8 @@ */ final class VarDumper implements Renderer { + private static ?DumpDescriptorInterface $describer = null; + public function isSupport(Frame $frame): bool { return $frame->type === ProtoType::VarDumper; @@ -32,6 +34,9 @@ public function render(OutputInterface $output, Frame $frame): void { \assert($frame instanceof Frame\VarDumper); + /** + * @var list{Data, array}|false $payload + */ $payload = @\unserialize(\base64_decode($frame->dump), ['allowed_classes' => [Data::class, Stub::class]]); // Impossible to decode the message, give up. @@ -39,19 +44,18 @@ public function render(OutputInterface $output, Frame $frame): void throw new \RuntimeException("Unable to decode a message."); } - static $describer = null; - $describer ??= $this->getDescriber(); + self::$describer ??= $this->getDescriber(); [$data, $context] = $payload; - $describer->describe(new SymfonyStyle(new ArrayInput([]), $output), $data, $context, 0); + self::$describer->describe(new SymfonyStyle(new ArrayInput([]), $output), $data, $context, 0); } private function getDescriber(): DumpDescriptorInterface { return new class() implements DumpDescriptorInterface { public function __construct( - private CliDumper $dumper = new CliDumper(), + private readonly CliDumper $dumper = new CliDumper(), ) {} /** @@ -72,8 +76,8 @@ public function describe(OutputInterface $output, Data $data, array $context, in \assert(\is_array($source)); $sourceInfo = \sprintf('%s:%d', $source['name'], $source['line']); - if ($fileLink = $source['file_link'] ?? null) { - $sourceInfo = \sprintf('%s', $fileLink, $sourceInfo); + if (isset($source['file_link'])) { + $sourceInfo = \sprintf('%s', $source['file_link'], $sourceInfo); $meta['Source'] = $sourceInfo; } diff --git a/src/Sender/Console/Support/Tables.php b/src/Sender/Console/Support/Tables.php index ae0ad2af..959c4cd0 100644 --- a/src/Sender/Console/Support/Tables.php +++ b/src/Sender/Console/Support/Tables.php @@ -15,6 +15,9 @@ */ final class Tables { + /** + * @param array $data + */ public static function renderKeyValueTable(OutputInterface $output, string $title, array $data): void { $table = (new Table($output))->setHeaderTitle($title); @@ -27,10 +30,10 @@ public static function renderKeyValueTable(OutputInterface $output, string $titl $valueLength = \max(1, (new Terminal())->getWidth() - 7 - $keyLength); $table->setRows([...(static function (array $data) use ($valueLength): iterable { + /** + * @var array $data + */ foreach ($data as $key => $value) { - if (!\is_string($value)) { - $value = Json::encode($value); - } $values = \strlen($value) > $valueLength ? \str_split($value, $valueLength) : [$value]; diff --git a/src/Support/Stream/Base64DecodeFilter.php b/src/Support/Stream/Base64DecodeFilter.php index 3d8cd3ec..fa5c4adc 100644 --- a/src/Support/Stream/Base64DecodeFilter.php +++ b/src/Support/Stream/Base64DecodeFilter.php @@ -52,7 +52,7 @@ public function filter($in, $out, &$consumed, bool $closing): int } // Decode part of the data - $bucket->data = \base64_decode($this->buffer . \substr($bucket->data, 0, -$d)); + $bucket->data = \base64_decode($this->buffer . \substr($bucket->data, 0, -$d), true); $consumed += $bucket->datalen; $this->buffer = \substr($bucket->data, -$d); diff --git a/src/Traffic/Message/Multipart/Part.php b/src/Traffic/Message/Multipart/Part.php index 1a6c64f8..e3ebf0d9 100644 --- a/src/Traffic/Message/Multipart/Part.php +++ b/src/Traffic/Message/Multipart/Part.php @@ -28,10 +28,11 @@ protected function __construct( */ public static function create(array $headers): Part { - $contentDisposition = self::findHeader($headers, 'Content-Disposition')[0] ?? null; + $contentDisposition = self::findHeader($headers, 'Content-Disposition')[0] ?? ''; $name = $fileName = null; - if ((string) $contentDisposition !== '') { + + if ($contentDisposition !== '') { // Get field name and file name $name = \preg_match('/\bname=(?:(?[^" ;,]++)|"(?[^"]++)")/', $contentDisposition, $matches) === 1 ? ($matches['a'] ?: $matches['b']) diff --git a/src/Traffic/Parser/Http.php b/src/Traffic/Parser/Http.php index 3441895e..815223a9 100644 --- a/src/Traffic/Parser/Http.php +++ b/src/Traffic/Parser/Http.php @@ -4,6 +4,7 @@ namespace Buggregator\Trap\Traffic\Parser; +use Buggregator\Trap\Support\Stream\Base64DecodeFilter; use Buggregator\Trap\Support\StreamHelper; use Buggregator\Trap\Traffic\Message\Multipart\Field; use Buggregator\Trap\Traffic\Message\Multipart\File; @@ -111,7 +112,9 @@ public static function parseMultipartBody(StreamInterface $stream, string $bound $writeFilters = []; if ($part->hasHeader('Content-Transfer-Encoding')) { $encoding = $part->getHeaderLine('Content-Transfer-Encoding'); - $encoding === 'base64' and $writeFilters[] = \Buggregator\Trap\Support\Stream\Base64DecodeFilter::FILTER_NAME; + if ($encoding === 'base64') { + $writeFilters[] = Base64DecodeFilter::FILTER_NAME; + } } $fileStream = StreamHelper::createFileStream(writeFilters: $writeFilters); From e41d505c565ad016ed0e3d04bcd723ecfae8a23b Mon Sep 17 00:00:00 2001 From: roxblnfk Date: Sat, 29 Jun 2024 23:03:19 +0400 Subject: [PATCH 04/21] style: fix code style --- src/Client/Caster/ProtobufCaster.php | 4 +--- src/Sender/Console/Renderer/VarDumper.php | 12 +++--------- src/Sender/Console/Support/Tables.php | 4 +--- src/Traffic/Parser/Http.php | 4 +--- 4 files changed, 6 insertions(+), 18 deletions(-) diff --git a/src/Client/Caster/ProtobufCaster.php b/src/Client/Caster/ProtobufCaster.php index 58393b14..f55ea27e 100644 --- a/src/Client/Caster/ProtobufCaster.php +++ b/src/Client/Caster/ProtobufCaster.php @@ -55,9 +55,7 @@ final class ProtobufCaster public static function cast(Message $c, array $a, Stub $stub, bool $isNested): array { - /** - * @var DescriptorPool $pool - */ + /** @var DescriptorPool $pool */ $pool = DescriptorPool::getGeneratedPool(); /** @var PublicDescriptor|InternalDescriptor $descriptor */ $descriptor = $pool->getDescriptorByClassName($c::class); diff --git a/src/Sender/Console/Renderer/VarDumper.php b/src/Sender/Console/Renderer/VarDumper.php index 7bc3f390..821e770f 100644 --- a/src/Sender/Console/Renderer/VarDumper.php +++ b/src/Sender/Console/Renderer/VarDumper.php @@ -34,17 +34,11 @@ public function render(OutputInterface $output, Frame $frame): void { \assert($frame instanceof Frame\VarDumper); - /** - * @var list{Data, array}|false $payload - */ - $payload = @\unserialize(\base64_decode($frame->dump), ['allowed_classes' => [Data::class, Stub::class]]); + /** @var array{Data, array}|false $payload */ + $payload = @\unserialize(\base64_decode($frame->dump, true), ['allowed_classes' => [Data::class, Stub::class]]); // Impossible to decode the message, give up. - if ($payload === false) { - throw new \RuntimeException("Unable to decode a message."); - } - - self::$describer ??= $this->getDescriber(); + $payload === false and throw new \RuntimeException("Unable to decode the message."); [$data, $context] = $payload; diff --git a/src/Sender/Console/Support/Tables.php b/src/Sender/Console/Support/Tables.php index 959c4cd0..57bc5c44 100644 --- a/src/Sender/Console/Support/Tables.php +++ b/src/Sender/Console/Support/Tables.php @@ -30,9 +30,7 @@ public static function renderKeyValueTable(OutputInterface $output, string $titl $valueLength = \max(1, (new Terminal())->getWidth() - 7 - $keyLength); $table->setRows([...(static function (array $data) use ($valueLength): iterable { - /** - * @var array $data - */ + /** @var array $data */ foreach ($data as $key => $value) { $values = \strlen($value) > $valueLength ? \str_split($value, $valueLength) diff --git a/src/Traffic/Parser/Http.php b/src/Traffic/Parser/Http.php index 815223a9..18efadda 100644 --- a/src/Traffic/Parser/Http.php +++ b/src/Traffic/Parser/Http.php @@ -112,9 +112,7 @@ public static function parseMultipartBody(StreamInterface $stream, string $bound $writeFilters = []; if ($part->hasHeader('Content-Transfer-Encoding')) { $encoding = $part->getHeaderLine('Content-Transfer-Encoding'); - if ($encoding === 'base64') { - $writeFilters[] = Base64DecodeFilter::FILTER_NAME; - } + $encoding === 'base64' and $writeFilters[] = Base64DecodeFilter::FILTER_NAME; } $fileStream = StreamHelper::createFileStream(writeFilters: $writeFilters); From b3ec1613ad5531c3c0d427e7ddd11bfdec4a9bf1 Mon Sep 17 00:00:00 2001 From: roxblnfk Date: Sat, 29 Jun 2024 23:04:34 +0400 Subject: [PATCH 05/21] fix(var-dumper):cli renderer must use provided output that may be buffered --- src/Sender/Console/Renderer/Sentry/Exceptions.php | 2 +- src/Sender/Console/Renderer/VarDumper.php | 13 ++++++------- src/Sender/Console/Support/Tables.php | 1 - 3 files changed, 7 insertions(+), 9 deletions(-) diff --git a/src/Sender/Console/Renderer/Sentry/Exceptions.php b/src/Sender/Console/Renderer/Sentry/Exceptions.php index 138acfcb..66b467f6 100644 --- a/src/Sender/Console/Renderer/Sentry/Exceptions.php +++ b/src/Sender/Console/Renderer/Sentry/Exceptions.php @@ -92,7 +92,7 @@ private static function renderTrace(OutputInterface $output, array $frames, bool "%s%s%s\n%s%s%s()", \str_pad("#$i", $numPad, ' '), (string) $file, - $line !== '' ? ":$line": '', + $line !== '' ? ":$line" : '', \str_repeat(' ', $numPad), $class, (string) $function, diff --git a/src/Sender/Console/Renderer/VarDumper.php b/src/Sender/Console/Renderer/VarDumper.php index 821e770f..98a2948f 100644 --- a/src/Sender/Console/Renderer/VarDumper.php +++ b/src/Sender/Console/Renderer/VarDumper.php @@ -23,8 +23,6 @@ */ final class VarDumper implements Renderer { - private static ?DumpDescriptorInterface $describer = null; - public function isSupport(Frame $frame): bool { return $frame->type === ProtoType::VarDumper; @@ -41,15 +39,16 @@ public function render(OutputInterface $output, Frame $frame): void $payload === false and throw new \RuntimeException("Unable to decode the message."); [$data, $context] = $payload; - - self::$describer->describe(new SymfonyStyle(new ArrayInput([]), $output), $data, $context, 0); + $this + ->getDescriber($output) + ->describe(new SymfonyStyle(new ArrayInput([]), $output), $data, $context, 0); } - private function getDescriber(): DumpDescriptorInterface + private function getDescriber(OutputInterface $output): DumpDescriptorInterface { - return new class() implements DumpDescriptorInterface { + return new class(new CliDumper($output)) implements DumpDescriptorInterface { public function __construct( - private readonly CliDumper $dumper = new CliDumper(), + private readonly CliDumper $dumper, ) {} /** diff --git a/src/Sender/Console/Support/Tables.php b/src/Sender/Console/Support/Tables.php index 57bc5c44..42c9a716 100644 --- a/src/Sender/Console/Support/Tables.php +++ b/src/Sender/Console/Support/Tables.php @@ -4,7 +4,6 @@ namespace Buggregator\Trap\Sender\Console\Support; -use Buggregator\Trap\Support\Json; use Symfony\Component\Console\Helper\Table; use Symfony\Component\Console\Output\OutputInterface; use Symfony\Component\Console\Terminal; From b3dc89d13f7583e8c0e6f4f04a2a11e7dcc7faf1 Mon Sep 17 00:00:00 2001 From: Plakhotnikov Vladimir Date: Sun, 30 Jun 2024 10:38:08 +0300 Subject: [PATCH 06/21] ci: add rector workflow --- .github/workflows/refactoring.yml | 67 +++++++++++++++++++ .github/workflows/static-analysis.yml | 8 +-- .php-cs-fixer.dist.php | 2 +- composer.json | 1 + composer.lock | 63 ++++++++++++++++- rector.php | 54 +++++++++++++++ src/Client/Caster/ProtobufCaster.php | 9 ++- src/Client/Caster/Trace.php | 2 +- src/Client/Caster/TraceFile.php | 2 +- src/Client/TrapHandle.php | 6 +- .../TrapHandle/ContextProvider/Source.php | 26 ++----- src/Client/TrapHandle/Dumper.php | 3 +- 12 files changed, 207 insertions(+), 36 deletions(-) create mode 100644 .github/workflows/refactoring.yml create mode 100644 rector.php diff --git a/.github/workflows/refactoring.yml b/.github/workflows/refactoring.yml new file mode 100644 index 00000000..df353c4d --- /dev/null +++ b/.github/workflows/refactoring.yml @@ -0,0 +1,67 @@ +--- + +on: # yamllint disable-line rule:truthy + pull_request: + paths: + - 'src/**' + - 'tests/**' + - 'composer.*' + push: + paths: + - 'src/**' + - 'tests/**' + - 'composer.*' + +name: ⚙️ Refactoring + +jobs: + rector: + timeout-minutes: 4 + runs-on: ${{ matrix.os }} + concurrency: + cancel-in-progress: true + group: rector-${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} + strategy: + fail-fast: true + matrix: + os: + - ubuntu-latest + php-version: + - '8.2' + dependencies: + - locked + steps: + - name: 📦 Check out the codebase + uses: actions/checkout@v4.1.7 + + - name: 🛠️ Setup PHP + uses: shivammathur/setup-php@2.30.4 + with: + php-version: ${{ matrix.php-version }} + extensions: none, ctype, curl, dom, json, mbstring, phar, simplexml, tokenizer, xml, xmlwriter, sockets, opcache, pcntl, posix + ini-values: error_reporting=E_ALL + coverage: none + + - name: 🛠️ Setup problem matchers + run: echo "::add-matcher::${{ runner.tool_cache }}/php.json" + + - name: 🤖 Validate composer.json and composer.lock + run: composer validate --ansi --strict + + - name: 🔍 Get composer cache directory + uses: wayofdev/gh-actions/actions/composer/get-cache-directory@v3.1.0 + + - name: ♻️ Restore cached dependencies installed with composer + uses: actions/cache@v4 + with: + path: ${{ env.COMPOSER_CACHE_DIR }} + key: php-${{ matrix.php-version }}-composer-${{ matrix.dependencies }}-${{ hashFiles('composer.lock') }} + restore-keys: php-${{ matrix.php-version }}-composer-${{ matrix.dependencies }}- + + - name: 📥 Install "${{ matrix.dependencies }}" dependencies + uses: wayofdev/gh-actions/actions/composer/install@v3.1.0 + with: + dependencies: ${{ matrix.dependencies }} + + - name: 🔍 Run static analysis using rector/rector + run: composer refactor:ci diff --git a/.github/workflows/static-analysis.yml b/.github/workflows/static-analysis.yml index b167f425..adc85429 100644 --- a/.github/workflows/static-analysis.yml +++ b/.github/workflows/static-analysis.yml @@ -38,7 +38,7 @@ jobs: - locked steps: - name: 📦 Check out the codebase - uses: actions/checkout@v4.1.5 + uses: actions/checkout@v4 - name: 🛠️ Setup PHP uses: shivammathur/setup-php@2.30.4 @@ -58,7 +58,7 @@ jobs: uses: wayofdev/gh-actions/actions/composer/get-cache-directory@v3.1.0 - name: ♻️ Restore cached dependencies installed with composer - uses: actions/cache@v4.0.2 + uses: actions/cache@v4 with: path: ${{ env.COMPOSER_CACHE_DIR }} key: php-${{ matrix.php-version }}-composer-${{ matrix.dependencies }}-${{ hashFiles('composer.lock') }} @@ -89,7 +89,7 @@ jobs: - locked steps: - name: 📦 Check out the codebase - uses: actions/checkout@v4.1.5 + uses: actions/checkout@v4 - name: 🛠️ Setup PHP uses: shivammathur/setup-php@2.30.4 @@ -109,7 +109,7 @@ jobs: uses: wayofdev/gh-actions/actions/composer/get-cache-directory@v3.1.0 - name: ♻️ Restore cached dependencies installed with composer - uses: actions/cache@v4.0.2 + uses: actions/cache@v4 with: path: ${{ env.COMPOSER_CACHE_DIR }} key: php-${{ matrix.php-version }}-composer-${{ matrix.dependencies }}-${{ hashFiles('composer.lock') }} diff --git a/.php-cs-fixer.dist.php b/.php-cs-fixer.dist.php index 56f76207..90758b8c 100644 --- a/.php-cs-fixer.dist.php +++ b/.php-cs-fixer.dist.php @@ -14,7 +14,7 @@ ->exclude([ __DIR__ . '/src/Test/Proto', ]) - ->addFiles([__FILE__]) + ->addFiles([__FILE__, __DIR__ . '/rector.php']) ->getConfig(); $config->setCacheFile(__DIR__ . '/.build/php-cs-fixer/php-cs-fixer.cache'); diff --git a/composer.json b/composer.json index ef78a3d0..1ac01480 100644 --- a/composer.json +++ b/composer.json @@ -67,6 +67,7 @@ "phpstan/phpstan-phpunit": "^1.3", "phpstan/phpstan-strict-rules": "^1.5", "phpunit/phpunit": "^10.5", + "rector/rector": "^1.1", "roxblnfk/unpoly": "^1.8.1", "vimeo/psalm": "^5.11", "wayofdev/cs-fixer-config": "^1.4" diff --git a/composer.lock b/composer.lock index b64091b2..bcc2ec45 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "c63c5b3a7fb9968c9a33be396d75e976", + "content-hash": "b6eac7c3997133078d6b6ff600095717", "packages": [ { "name": "clue/stream-filter", @@ -4644,6 +4644,65 @@ ], "time": "2023-06-16T10:52:11+00:00" }, + { + "name": "rector/rector", + "version": "1.1.1", + "source": { + "type": "git", + "url": "https://github.com/rectorphp/rector.git", + "reference": "c930cdb21294f10955ddfc31b720971e8333943d" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/rectorphp/rector/zipball/c930cdb21294f10955ddfc31b720971e8333943d", + "reference": "c930cdb21294f10955ddfc31b720971e8333943d", + "shasum": "" + }, + "require": { + "php": "^7.2|^8.0", + "phpstan/phpstan": "^1.11" + }, + "conflict": { + "rector/rector-doctrine": "*", + "rector/rector-downgrade-php": "*", + "rector/rector-phpunit": "*", + "rector/rector-symfony": "*" + }, + "suggest": { + "ext-dom": "To manipulate phpunit.xml via the custom-rule command" + }, + "bin": [ + "bin/rector" + ], + "type": "library", + "autoload": { + "files": [ + "bootstrap.php" + ] + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "description": "Instant Upgrade and Automated Refactoring of any PHP code", + "keywords": [ + "automation", + "dev", + "migration", + "refactoring" + ], + "support": { + "issues": "https://github.com/rectorphp/rector/issues", + "source": "https://github.com/rectorphp/rector/tree/1.1.1" + }, + "funding": [ + { + "url": "https://github.com/tomasvotruba", + "type": "github" + } + ], + "time": "2024-06-21T07:51:17+00:00" + }, { "name": "roxblnfk/unpoly", "version": "1.8.1.1", @@ -6530,5 +6589,5 @@ "platform-overrides": { "php": "8.1.27" }, - "plugin-api-version": "2.6.0" + "plugin-api-version": "2.3.0" } diff --git a/rector.php b/rector.php new file mode 100644 index 00000000..7dd0a781 --- /dev/null +++ b/rector.php @@ -0,0 +1,54 @@ +withPaths([ + // let's add more directories step by step + // __DIR__ . '/src', + // __DIR__ . '/tests', + // __DIR__ . '/bin', + __DIR__ . '/src/Client', + ]) + ->withSkipPath('src/Client/TrapHandle/ContextProvider/Source.php') + ->withPHPStanConfigs([ + __DIR__ . '/phpstan-baseline.neon', + ]) + ->withImportNames(importNames: true, importDocBlockNames: true, importShortClasses: false, removeUnusedImports: true) + ->withPhpVersion(PhpVersion::PHP_81) + ->withPhpSets(php81: true) + ->withPreparedSets( + deadCode: false, + codeQuality: true, + codingStyle: true, + typeDeclarations: true, + privatization: true, + naming: false, + instanceOf: true, + earlyReturn: true, + strictBooleans: true, + carbon: false, + rectorPreset: true, + )->withSkip([ + InlineArrayReturnAssignRector::class, + PostIncDecToPreIncDecRector::class, + InlineIfToExplicitIfRector::class, + LogicalToBooleanRector::class, + BinaryOpNullableToInstanceofRector::class, + FlipTypeControlToUseExclusiveTypeRector::class, + DisallowedEmptyRuleFixerRector::class, + NullToStrictStringFuncCallArgRector::class, + EncapsedStringsToSprintfRector::class, + ]); diff --git a/src/Client/Caster/ProtobufCaster.php b/src/Client/Caster/ProtobufCaster.php index 05dd8f4b..0912931d 100644 --- a/src/Client/Caster/ProtobufCaster.php +++ b/src/Client/Caster/ProtobufCaster.php @@ -4,6 +4,9 @@ namespace Buggregator\Trap\Client\Caster; +use Google\Protobuf\Internal\FieldDescriptor; +use Google\Protobuf\Internal\EnumDescriptor; +use Google\Protobuf\Internal\EnumValueDescriptorProto; use Google\Protobuf\Descriptor as PublicDescriptor; use Google\Protobuf\Internal\Descriptor as InternalDescriptor; use Google\Protobuf\Internal\DescriptorPool; @@ -118,7 +121,7 @@ private static function extractViaInternal(Message $message, InternalDescriptor $values = []; for ($i = 0; $i < $pub->getFieldCount(); $i++) { - /** @var \Google\Protobuf\Internal\FieldDescriptor $fd */ + /** @var FieldDescriptor $fd */ $fd = $descriptor->getFieldByIndex($i); $value = $message->{$fd->getGetter()}(); @@ -150,9 +153,9 @@ private static function extractViaInternal(Message $message, InternalDescriptor // Wrap ENUM if ($fd->getType() === GPBType::ENUM) { - /** @var \Google\Protobuf\Internal\EnumDescriptor $ed */ + /** @var EnumDescriptor $ed */ $ed = $fd->getEnumType(); - /** @var \Google\Protobuf\Internal\EnumValueDescriptorProto $v */ + /** @var EnumValueDescriptorProto $v */ $v = $ed->getValueByNumber($value); $values[$fd->getName()] = new EnumValue( diff --git a/src/Client/Caster/Trace.php b/src/Client/Caster/Trace.php index 9be5c575..1571e11f 100644 --- a/src/Client/Caster/Trace.php +++ b/src/Client/Caster/Trace.php @@ -10,7 +10,7 @@ * @see tr() * @internal */ -final class Trace +final class Trace implements \Stringable { /** * @param int<0, max> $number The tick number. diff --git a/src/Client/Caster/TraceFile.php b/src/Client/Caster/TraceFile.php index 0447750c..b06f6279 100644 --- a/src/Client/Caster/TraceFile.php +++ b/src/Client/Caster/TraceFile.php @@ -7,7 +7,7 @@ /** * @internal */ -final class TraceFile +final class TraceFile implements \Stringable { /** * @param array{ diff --git a/src/Client/TrapHandle.php b/src/Client/TrapHandle.php index f4ba6edf..126e7d52 100644 --- a/src/Client/TrapHandle.php +++ b/src/Client/TrapHandle.php @@ -23,7 +23,7 @@ final class TrapHandle private int $depth = 0; - private StaticState $staticState; + private readonly StaticState $staticState; private function __construct( private array $values, @@ -139,7 +139,7 @@ public function once(): self */ public function return(int|string $key = 0): mixed { - if (\count($this->values) === 0) { + if ($this->values === []) { throw new \InvalidArgumentException('No values to return.'); } @@ -176,8 +176,6 @@ public function return(int|string $key = 0): mixed * ```php * trap()->context(['foo bar', => 42, 'baz' => 69]); * ``` - * - * @param mixed ...$values */ public function context(mixed ...$values): self { diff --git a/src/Client/TrapHandle/ContextProvider/Source.php b/src/Client/TrapHandle/ContextProvider/Source.php index 100d4d6e..9ca0f349 100644 --- a/src/Client/TrapHandle/ContextProvider/Source.php +++ b/src/Client/TrapHandle/ContextProvider/Source.php @@ -27,24 +27,10 @@ */ final class Source implements ContextProviderInterface { - private int $limit; - - private ?string $charset; - - private ?string $projectDir; - - private ?FileLinkFormatter $fileLinkFormatter; - /** * @psalm-suppress UndefinedClass */ - public function __construct(string $charset = null, string $projectDir = null, FileLinkFormatter $fileLinkFormatter = null, int $limit = 9) - { - $this->charset = $charset; - $this->projectDir = $projectDir; - $this->fileLinkFormatter = $fileLinkFormatter; - $this->limit = $limit; - } + public function __construct(private readonly ?string $charset = null, private ?string $projectDir = null, private readonly ?FileLinkFormatter $fileLinkFormatter = null, private readonly int $limit = 9) {} public function getContext(): ?array { @@ -80,7 +66,7 @@ public function getContext(): ?array $file = \method_exists($template, 'getSourceContext') ? $template->getSourceContext()->getPath() : null; if ($src) { - $src = \explode("\n", $src); + $src = \explode("\n", (string) $src); $fileExcerpt = []; for ($i = \max($line - 3, 1), $max = \min($line + 3, \count($src)); $i <= $max; ++$i) { @@ -90,9 +76,11 @@ public function getContext(): ?array $fileExcerpt = '
    ' . \implode("\n", $fileExcerpt) . '
'; } } + break; } } + break; } } @@ -107,8 +95,8 @@ public function getContext(): ?array if ($this->projectDir !== null) { $context['project_dir'] = $this->projectDir; - if (\str_starts_with($file, $this->projectDir)) { - $context['file_relative'] = \ltrim(\substr($file, \strlen($this->projectDir)), \DIRECTORY_SEPARATOR); + if (\str_starts_with((string) $file, $this->projectDir)) { + $context['file_relative'] = \ltrim(\substr((string) $file, \strlen($this->projectDir)), \DIRECTORY_SEPARATOR); } } @@ -123,7 +111,7 @@ private function htmlEncode(string $s): string { $html = ''; - $dumper = new HtmlDumper(static function ($line) use (&$html): void { $html .= $line; }, $this->charset); + $dumper = new HtmlDumper(static function (string $line) use (&$html): void { $html .= $line; }, $this->charset); $dumper->setDumpHeader(''); $dumper->setDumpBoundaries('', ''); diff --git a/src/Client/TrapHandle/Dumper.php b/src/Client/TrapHandle/Dumper.php index 10e662f6..d3726ab0 100644 --- a/src/Client/TrapHandle/Dumper.php +++ b/src/Client/TrapHandle/Dumper.php @@ -4,6 +4,7 @@ namespace Buggregator\Trap\Client\TrapHandle; +use Buggregator\Trap\Client\TrapHandle\ContextProvider\Source; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\RequestStack; use Symfony\Component\HttpKernel\Debug\FileLinkFormatter; @@ -132,7 +133,7 @@ private static function getContextProviders(): array return $contextProviders + [ 'cli' => new CliContextProvider(), - 'source' => new ContextProvider\Source(null, null, $fileLinkFormatter), + 'source' => new Source(null, null, $fileLinkFormatter), ]; } } From 4269e352777d4f266881f79fef2d62231fe2ca95 Mon Sep 17 00:00:00 2001 From: Plakhotnikov Vladimir Date: Mon, 8 Jul 2024 22:22:35 +0300 Subject: [PATCH 07/21] ci: add rector workflow --- rector.php | 2 +- src/Client/TrapHandle/Dumper.php | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/rector.php b/rector.php index 7dd0a781..613b37b1 100644 --- a/rector.php +++ b/rector.php @@ -26,7 +26,7 @@ ->withPHPStanConfigs([ __DIR__ . '/phpstan-baseline.neon', ]) - ->withImportNames(importNames: true, importDocBlockNames: true, importShortClasses: false, removeUnusedImports: true) + ->withImportNames(importNames: false, importDocBlockNames: true, importShortClasses: false, removeUnusedImports: true) ->withPhpVersion(PhpVersion::PHP_81) ->withPhpSets(php81: true) ->withPreparedSets( diff --git a/src/Client/TrapHandle/Dumper.php b/src/Client/TrapHandle/Dumper.php index d3726ab0..10e662f6 100644 --- a/src/Client/TrapHandle/Dumper.php +++ b/src/Client/TrapHandle/Dumper.php @@ -4,7 +4,6 @@ namespace Buggregator\Trap\Client\TrapHandle; -use Buggregator\Trap\Client\TrapHandle\ContextProvider\Source; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\RequestStack; use Symfony\Component\HttpKernel\Debug\FileLinkFormatter; @@ -133,7 +132,7 @@ private static function getContextProviders(): array return $contextProviders + [ 'cli' => new CliContextProvider(), - 'source' => new Source(null, null, $fileLinkFormatter), + 'source' => new ContextProvider\Source(null, null, $fileLinkFormatter), ]; } } From 9b0c1ac65ed53dd4615584ab5885aa0d2437b65a Mon Sep 17 00:00:00 2001 From: Plakhotnikov Vladimir Date: Sun, 14 Jul 2024 09:40:16 +0300 Subject: [PATCH 08/21] ci: add rector workflow --- .github/workflows/static-analysis.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/static-analysis.yml b/.github/workflows/static-analysis.yml index adc85429..f6717971 100644 --- a/.github/workflows/static-analysis.yml +++ b/.github/workflows/static-analysis.yml @@ -38,7 +38,7 @@ jobs: - locked steps: - name: 📦 Check out the codebase - uses: actions/checkout@v4 + uses: actions/checkout@v4.1.5 - name: 🛠️ Setup PHP uses: shivammathur/setup-php@2.30.4 @@ -89,7 +89,7 @@ jobs: - locked steps: - name: 📦 Check out the codebase - uses: actions/checkout@v4 + uses: actions/checkout@v4.1.5 - name: 🛠️ Setup PHP uses: shivammathur/setup-php@2.30.4 From 17f299e2e93c3492c75537f2c6f6be565737903e Mon Sep 17 00:00:00 2001 From: Aleksei Gagarin Date: Wed, 24 Jul 2024 09:10:10 +0400 Subject: [PATCH 09/21] docs: add hidden contributors image --- README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/README.md b/README.md index 64cdaed4..9d12d98a 100644 --- a/README.md +++ b/README.md @@ -251,6 +251,8 @@ Buggregator Trap is open-sourced software licensed under the BSD-3 license.