diff --git a/src/FrameBuilder.php b/src/FrameBuilder.php index 968b80fce..a2b83751e 100644 --- a/src/FrameBuilder.php +++ b/src/FrameBuilder.php @@ -4,6 +4,7 @@ namespace Sentry; +use Sentry\DataCollection\KeyValueDataFilter; use Sentry\Serializer\RepresentationSerializerInterface; use Sentry\Util\PrefixStripper; @@ -201,6 +202,15 @@ private function getFunctionArguments(array $backtraceFrame): array } } + $dataCollection = $this->options->getDataCollection(); + + if ($dataCollection !== null) { + $argumentValues = KeyValueDataFilter::filterKeyValueData( + $argumentValues, + $dataCollection->getStackFrameVariables() + ) ?? []; + } + foreach ($argumentValues as $argumentName => $argumentValue) { $argumentValues[$argumentName] = $this->representationSerializer->representationSerialize($argumentValue); } diff --git a/src/Integration/FrameContextifierIntegration.php b/src/Integration/FrameContextifierIntegration.php index f0ff6f59b..f3b3f104e 100644 --- a/src/Integration/FrameContextifierIntegration.php +++ b/src/Integration/FrameContextifierIntegration.php @@ -47,7 +47,11 @@ public function setupOnce(): void return $event; } - $maxContextLines = $client->getOptions()->getContextLines(); + $options = $client->getOptions(); + $dataCollection = $options->getDataCollection(); + $maxContextLines = $dataCollection === null + ? $options->getContextLines() + : $dataCollection->getFrameContextLines(); $integration = $client->getIntegration(self::class); if ($integration === null || $maxContextLines === null) { diff --git a/tests/Integration/FrameContextifierIntegrationTest.php b/tests/Integration/FrameContextifierIntegrationTest.php index 5667826b0..ff50550e5 100644 --- a/tests/Integration/FrameContextifierIntegrationTest.php +++ b/tests/Integration/FrameContextifierIntegrationTest.php @@ -23,9 +23,21 @@ final class FrameContextifierIntegrationTest extends TestCase /** * @dataProvider invokeDataProvider */ - public function testInvoke(string $fixtureFilePath, int $lineNumber, int $contextLines, int $preContextCount, int $postContextCount): void - { - $options = new Options(['context_lines' => $contextLines]); + public function testInvoke( + string $fixtureFilePath, + int $lineNumber, + int $contextLines, + int $preContextCount, + int $postContextCount, + ?int $dataCollectionContextLines = null + ): void { + $options = ['context_lines' => $contextLines]; + + if ($dataCollectionContextLines !== null) { + $options['data_collection'] = ['frame_context_lines' => $dataCollectionContextLines]; + } + + $options = new Options($options); $integration = new FrameContextifierIntegration(); $integration->setupOnce(); @@ -108,6 +120,24 @@ public static function invokeDataProvider(): \Generator 2, 5, ]; + + yield 'data collection context lines take precedence over legacy option' => [ + realpath(__DIR__ . '/../Fixtures/code/LongFile.php'), + 8, + 1, + 3, + 3, + 3, + ]; + + yield 'data collection can omit surrounding context lines' => [ + realpath(__DIR__ . '/../Fixtures/code/LongFile.php'), + 8, + 5, + 0, + 0, + 0, + ]; } public function testInvokeLogsWarningMessageIfSourceCodeExcerptCannotBeRetrievedForFrame(): void diff --git a/tests/StacktraceBuilderTest.php b/tests/StacktraceBuilderTest.php index 544919063..a17e0c161 100644 --- a/tests/StacktraceBuilderTest.php +++ b/tests/StacktraceBuilderTest.php @@ -50,4 +50,193 @@ public function testBuildFromBacktrace(): void $this->assertSame(__FILE__, $frames[2]->getAbsoluteFilePath()); $this->assertSame($expectedLine, $frames[2]->getLine()); } + + /** + * @dataProvider realExceptionStackFrameVariablesDataProvider + * + * @param array $options + * @param array> $expectedVariables + */ + public function testStackFrameVariablesFromRealException(array $options, array $expectedVariables): void + { + $previousIgnoreArgs = \ini_get('zend.exception_ignore_args'); + + try { + if ($previousIgnoreArgs !== false + && (ini_set('zend.exception_ignore_args', '0') === false + || \ini_get('zend.exception_ignore_args') !== '0')) { + $this->markTestSkipped('zend.exception_ignore_args cannot be disabled.'); + } + + $exception = self::createNestedException(); + $sdkOptions = new Options($options); + $stacktraceBuilder = new StacktraceBuilder( + $sdkOptions, + new RepresentationSerializer($sdkOptions) + ); + $frames = $stacktraceBuilder->buildFromException($exception)->getFrames(); + $actualVariables = []; + + foreach ($frames as $frame) { + $rawFunctionName = $frame->getRawFunctionName(); + + if ($rawFunctionName === null) { + continue; + } + + $separatorPosition = strrpos($rawFunctionName, '::'); + $methodName = $separatorPosition === false + ? $rawFunctionName + : substr($rawFunctionName, $separatorPosition + 2); + + if (\array_key_exists($methodName, $expectedVariables)) { + $actualVariables[$methodName] = $frame->getVars(); + } + } + + ksort($actualVariables); + ksort($expectedVariables); + + $this->assertSame($expectedVariables, $actualVariables); + } finally { + if ($previousIgnoreArgs !== false) { + ini_set('zend.exception_ignore_args', $previousIgnoreArgs); + } + } + } + + public static function realExceptionStackFrameVariablesDataProvider(): \Generator + { + yield 'legacy behavior is unchanged' => [ + [], + [ + 'stackFrameInner' => [ + 'apiToken' => 'nested-secret', + 'safeValue' => 'safe', + ], + 'stackFrameMiddle' => [ + 'metadata' => [ + 'api_token' => 'nested-secret', + 'name' => 'alice', + ], + ], + 'stackFrameOuter' => [ + 'requestId' => 'request-123', + 'password' => 'secret', + ], + ], + ]; + + yield 'default data collection filters mandatory sensitive values' => [ + ['data_collection' => []], + [ + 'stackFrameInner' => [ + 'apiToken' => '[Filtered]', + 'safeValue' => 'safe', + ], + 'stackFrameMiddle' => [ + 'metadata' => [ + 'api_token' => '[Filtered]', + 'name' => 'alice', + ], + ], + 'stackFrameOuter' => [ + 'requestId' => 'request-123', + 'password' => '[Filtered]', + ], + ], + ]; + + yield 'collection can be disabled with boolean shorthand' => [ + ['data_collection' => ['stack_frame_variables' => false]], + [ + 'stackFrameInner' => [], + 'stackFrameMiddle' => [], + 'stackFrameOuter' => [], + ], + ]; + + yield 'allow list filters values not matching configured terms' => [ + [ + 'data_collection' => [ + 'stack_frame_variables' => [ + 'mode' => 'allowList', + 'terms' => ['request'], + ], + ], + ], + [ + 'stackFrameInner' => [ + 'apiToken' => '[Filtered]', + 'safeValue' => '[Filtered]', + ], + 'stackFrameMiddle' => [ + 'metadata' => '[Filtered]', + ], + 'stackFrameOuter' => [ + 'requestId' => 'request-123', + 'password' => '[Filtered]', + ], + ], + ]; + + yield 'deny list combines mandatory and custom terms' => [ + [ + 'data_collection' => [ + 'stack_frame_variables' => [ + 'mode' => 'denyList', + 'terms' => ['request'], + ], + ], + ], + [ + 'stackFrameInner' => [ + 'apiToken' => '[Filtered]', + 'safeValue' => 'safe', + ], + 'stackFrameMiddle' => [ + 'metadata' => [ + 'api_token' => '[Filtered]', + 'name' => 'alice', + ], + ], + 'stackFrameOuter' => [ + 'requestId' => '[Filtered]', + 'password' => '[Filtered]', + ], + ], + ]; + } + + private static function createNestedException(): \RuntimeException + { + try { + self::stackFrameOuter('request-123', 'secret'); + } catch (\RuntimeException $exception) { + return $exception; + } + + throw new \LogicException('Expected the nested stack frame fixture to throw.'); + } + + private static function stackFrameOuter(string $requestId, string $password): void + { + self::stackFrameMiddle([ + 'api_token' => 'nested-secret', + 'name' => 'alice', + ]); + } + + /** + * @param array $metadata + */ + private static function stackFrameMiddle(array $metadata): void + { + self::stackFrameInner($metadata['api_token'], 'safe'); + } + + private static function stackFrameInner(string $apiToken, string $safeValue): void + { + throw new \RuntimeException('Real nested stack frame fixture.'); + } }