fix: invalid sample_rand is ignored (#2077)

Author: Litarnus

src/Tracing/Traits/TraceHeaderParserTrait.php

@@ -92,10 +92,11 @@ protected static function parseTraceAndBaggageHeaders(string $sentryTrace, strin
         }
 
         // Store the propagated trace sample rand or generate a new one
-        if ($samplingContext->has('sample_rand')) {
-            $result['sampleRand'] = (float) $samplingContext->get('sample_rand');
-        } else {
-            if ($samplingContext->has('sample_rate') && $result['parentSampled'] !== null) {
+        if ($hasSentryTrace) {
+            $incomingSampleRand = self::parseSampleRand($samplingContext);
+            if ($incomingSampleRand !== null) {
+                $result['sampleRand'] = $incomingSampleRand;
+            } elseif ($samplingContext->has('sample_rate') && $result['parentSampled'] !== null) {
                 if ($result['parentSampled'] === true) {
                     // [0, rate)
                     $result['sampleRand'] = round(mt_rand(0, mt_getrandmax() - 1) / mt_getrandmax() * (float) $samplingContext->get('sample_rate'), 6);
@@ -112,6 +113,32 @@ protected static function parseTraceAndBaggageHeaders(string $sentryTrace, strin
         return $result;
     }
 
+    private static function parseSampleRand(DynamicSamplingContext $samplingContext): ?float
+    {
+        $sampleRand = $samplingContext->get('sample_rand');
+        if ($sampleRand === null) {
+            return null;
+        }
+
+        if (is_numeric($sampleRand)) {
+            $sampleRandAsFloat = (float) $sampleRand;
+            if ($sampleRandAsFloat >= 0.0 && $sampleRandAsFloat < 1.0) {
+                return $sampleRandAsFloat;
+            }
+        }
+
+        $hub = SentrySdk::getCurrentHub();
+        $client = $hub->getClient();
+        if ($client !== null) {
+            $client->getOptions()->getLoggerOrNullLogger()->debug(
+                'Ignoring invalid sentry-sample_rand baggage value because it must be a numeric value in the range [0, 1).',
+                ['sample_rand' => $sampleRand]
+            );
+        }
+
+        return null;
+    }
+
     private static function shouldContinueTrace(DynamicSamplingContext $samplingContext): bool
     {
         $hub = SentrySdk::getCurrentHub();

tests/State/HubTest.php

@@ -760,6 +760,17 @@ public static function startTransactionDataProvider(): iterable
             false,
         ];
 
+        yield 'Invalid incoming sample_rand is ignored' => [
+            new Options([
+                'traces_sample_rate' => 1.0,
+            ]),
+            TransactionContext::fromHeaders(
+                '566e3688a61d4bc888951642d6f14a19-566e3688a61d4bc8',
+                'sentry-sample_rand=2.0'
+            ),
+            true,
+        ];
+
         yield 'Out of range sample rate returned from traces_sampler (lower than minimum)' => [
             new Options([
                 'traces_sampler' => static function (): float {

tests/Tracing/PropagationContextTest.php

@@ -136,6 +136,41 @@ public function testGetTraceContext(): void
         ], $propagationContext->getTraceContext());
     }
 
+    /**
+     * @dataProvider invalidSampleRandDataProvider
+     */
+    public function testInvalidSampleRandIsIgnored(string $sampleRand): void
+    {
+        $propagationContext = PropagationContext::fromHeaders(
+            '566e3688a61d4bc888951642d6f14a19-566e3688a61d4bc8-1',
+            'sentry-sample_rate=0.4,sentry-sample_rand=' . rawurlencode($sampleRand)
+        );
+
+        $generatedSampleRand = $propagationContext->getSampleRand();
+
+        $this->assertNotNull($generatedSampleRand);
+        $this->assertGreaterThanOrEqual(0.0, $generatedSampleRand);
+        $this->assertLessThan(0.4, $generatedSampleRand);
+    }
+
+    public function testSampleRandIsIgnoredWithoutSentryTraceHeader(): void
+    {
+        $propagationContext = PropagationContext::fromHeaders('', 'sentry-sample_rand=-1.0');
+        $sampleRand = $propagationContext->getSampleRand();
+
+        $this->assertNotNull($sampleRand);
+        $this->assertGreaterThanOrEqual(0.0, $sampleRand);
+        $this->assertLessThanOrEqual(1.0, $sampleRand);
+    }
+
+    public static function invalidSampleRandDataProvider(): iterable
+    {
+        yield ['-1.0'];
+        yield ['1'];
+        yield ['2.0'];
+        yield ['foo'];
+    }
+
     public function testSampleRandRangeWhenParentNotSampledAndSampleRateProvided(): void
     {
         $propagationContext = PropagationContext::fromHeaders(

tests/Tracing/TransactionContextTest.php

@@ -5,6 +5,11 @@
 namespace Sentry\Tests\Tracing;
 
 use PHPUnit\Framework\TestCase;
+use Psr\Log\LoggerInterface;
+use Sentry\ClientInterface;
+use Sentry\Options;
+use Sentry\SentrySdk;
+use Sentry\State\Hub;
 use Sentry\Tracing\DynamicSamplingContext;
 use Sentry\Tracing\SpanId;
 use Sentry\Tracing\TraceId;
@@ -135,6 +140,67 @@ public static function tracingDataProvider(): iterable
         ];
     }
 
+    /**
+     * @dataProvider invalidSampleRandDataProvider
+     */
+    public function testInvalidSampleRandIsIgnored(string $sampleRand): void
+    {
+        $context = TransactionContext::fromHeaders(
+            '566e3688a61d4bc888951642d6f14a19-566e3688a61d4bc8-1',
+            'sentry-sample_rate=0.4,sentry-sample_rand=' . rawurlencode($sampleRand)
+        );
+
+        $generatedSampleRand = $context->getMetadata()->getSampleRand();
+
+        $this->assertNotNull($generatedSampleRand);
+        $this->assertGreaterThanOrEqual(0.0, $generatedSampleRand);
+        $this->assertLessThan(0.4, $generatedSampleRand);
+    }
+
+    public function testSampleRandIsIgnoredWithoutSentryTraceHeader(): void
+    {
+        $context = TransactionContext::fromHeaders('', 'sentry-sample_rand=-1.0');
+        $sampleRand = $context->getMetadata()->getSampleRand();
+
+        $this->assertNotNull($sampleRand);
+        $this->assertGreaterThanOrEqual(0.0, $sampleRand);
+        $this->assertLessThanOrEqual(1.0, $sampleRand);
+    }
+
+    public function testInvalidSampleRandIsLogged(): void
+    {
+        $logger = $this->createMock(LoggerInterface::class);
+        $logger->expects($this->once())
+            ->method('debug')
+            ->with(
+                $this->stringContains('Ignoring invalid sentry-sample_rand baggage value'),
+                ['sample_rand' => '-1.0']
+            );
+
+        $client = $this->createMock(ClientInterface::class);
+        $client->method('getOptions')
+            ->willReturn(new Options(['logger' => $logger]));
+
+        SentrySdk::setCurrentHub(new Hub($client));
+
+        try {
+            TransactionContext::fromHeaders(
+                '566e3688a61d4bc888951642d6f14a19-566e3688a61d4bc8',
+                'sentry-sample_rand=-1.0'
+            );
+        } finally {
+            SentrySdk::setCurrentHub(new Hub());
+        }
+    }
+
+    public static function invalidSampleRandDataProvider(): iterable
+    {
+        yield ['-1.0'];
+        yield ['1'];
+        yield ['2.0'];
+        yield ['foo'];
+    }
+
     public function testSampleRandRangeWhenParentNotSampledAndSampleRateProvided(): void
     {
         $context = TransactionContext::fromHeaders(