Fixed "malformed UTF-8 characters" issue (#1120)

* Workaround potential "malformed UTF-8 characters" exception * Fixed unit tests
elastic · Jan 15, 2024 · 7a3f179 · 7a3f179
1 parent 10be2bd
commit 7a3f179
Show file tree

Hide file tree

Showing 10 changed files with 35 additions and 87 deletions.
diff --git a/agent/php/ElasticApm/Impl/Error.php b/agent/php/ElasticApm/Impl/Error.php
@@ -48,7 +48,7 @@ class Error implements SerializableDataInterface, LoggableInterface
      *
      * @link https://github.com/elastic/apm-server/blob/7.0/docs/spec/errors/error.json#L6
      */
-    public $timestamp;
+    private $timestamp;
 
     /**
      * @var string
@@ -71,7 +71,7 @@ class Error implements SerializableDataInterface, LoggableInterface
      *
      * @link https://github.com/elastic/apm-server/blob/7.0/docs/spec/errors/error.json#L14
      */
-    public $traceId = null;
+    private $traceId = null;
 
     /**
      * @var ?string
@@ -83,7 +83,7 @@ class Error implements SerializableDataInterface, LoggableInterface
      *
      * @link https://github.com/elastic/apm-server/blob/7.0/docs/spec/errors/error.json#L19
      */
-    public $transactionId = null;
+    private $transactionId = null;
 
     /**
      * @var ?string
@@ -95,7 +95,7 @@ class Error implements SerializableDataInterface, LoggableInterface
      *
      * @link https://github.com/elastic/apm-server/blob/7.0/docs/spec/errors/error.json#L24
      */
-    public $parentId = null;
+    private $parentId = null;
 
     /**
      * @var ?ErrorTransactionData
@@ -104,14 +104,14 @@ class Error implements SerializableDataInterface, LoggableInterface
      *
      * @link https://github.com/elastic/apm-server/blob/7.0/docs/spec/errors/error.json#L29
      */
-    public $transaction = null;
+    private $transaction = null;
 
     /**
      * @var ?TransactionContext
      *
      * @link https://github.com/elastic/apm-server/blob/7.0/docs/spec/errors/error.json#L44
      */
-    public $context = null;
+    private $context = null;
 
     /**
      * @var ?string
@@ -122,7 +122,7 @@ class Error implements SerializableDataInterface, LoggableInterface
      *
      * @link https://github.com/elastic/apm-server/blob/v7.0.0/docs/spec/errors/error.json#L47
      */
-    public $culprit = null;
+    private $culprit = null;
 
     /**
      * @var ?ErrorExceptionData
@@ -131,7 +131,7 @@ class Error implements SerializableDataInterface, LoggableInterface
      *
      * @link https://github.com/elastic/apm-server/blob/7.0/docs/spec/errors/error.json#L29
      */
-    public $exception = null;
+    private $exception = null;
 
     public static function build(Tracer $tracer, ErrorExceptionData $errorExceptionData, ?Transaction $transaction, ?Span $span): Error
     {

diff --git a/agent/php/ElasticApm/Impl/Span.php b/agent/php/ElasticApm/Impl/Span.php
@@ -41,22 +41,22 @@
 final class Span extends ExecutionSegment implements SpanInterface, SpanToSendInterface
 {
     /** @var string */
-    public $parentId;
+    private $parentId;
 
     /** @var string */
-    public $transactionId;
+    private $transactionId;
 
     /** @var ?string */
-    public $action = null;
+    private $action = null;
 
     /** @var ?string */
-    public $subtype = null;
+    private $subtype = null;
 
     /** @var null|StackTraceFrame[] */
-    public $stackTrace = null;
+    private $stackTrace = null;
 
     /** @var ?SpanContext */
-    public $context = null;
+    private $context = null;
 
     /** @var Logger */
     private $logger;

diff --git a/agent/php/ElasticApm/Impl/Transaction.php b/agent/php/ElasticApm/Impl/Transaction.php
@@ -49,22 +49,22 @@
 final class Transaction extends ExecutionSegment implements TransactionInterface
 {
     /** @var ?string */
-    public $parentId = null;
+    private $parentId = null;
 
     /** @var int */
     public $startedSpansCount = 0;
 
     /** @var int */
-    public $droppedSpansCount = 0;
+    private $droppedSpansCount = 0;
 
     /** @var ?string */
-    public $result = null;
+    private $result = null;
 
     /** @var bool */
-    public $isSampled;
+    private $isSampled;
 
     /** @var ?TransactionContext */
-    public $context = null;
+    private $context = null;
 
     /** @var Tracer */
     private $tracer;

diff --git a/agent/php/ElasticApm/Impl/Util/JsonUtil.php b/agent/php/ElasticApm/Impl/Util/JsonUtil.php
@@ -42,7 +42,7 @@ final class JsonUtil
      */
     public static function encode($data, bool $prettyPrint = false): string
     {
-        $options = 0;
+        $options = JSON_INVALID_UTF8_SUBSTITUTE;
         $options |= $prettyPrint ? JSON_PRETTY_PRINT : 0;
         $encodedData = json_encode($data, $options);
         if ($encodedData === false) {

diff --git a/tests/ElasticApmTests/UnitTests/PublicApiTest.php b/tests/ElasticApmTests/UnitTests/PublicApiTest.php
@@ -585,4 +585,19 @@ public function testNumericStringForLabelKey(): void
         self::assertSame('label_value_for_key_123', self::getLabel($reportedTx, '123'));
         self::assertSame('custom_value_for_key_456', self::getTransactionContextCustom($reportedTx, '456'));
     }
+
+    private const INVALID_UTF8_CHARACTER = "\xb0";
+    private const UNICODE_REPLACEMENT_CHARACTER_JSON_ENCODED = '\ufffd';
+
+    public function testMalformedUtf8(): void
+    {
+        // Act
+        $tx = $this->tracer->beginTransaction('test_TX_name_[' . self::INVALID_UTF8_CHARACTER . ']', 'test_TX_type');
+        $tx->end();
+
+        // Assert
+        $reportedTx = $this->mockEventSink->singleTransaction();
+        $this->assertSame('test_TX_name_[' . json_decode('"' . self::UNICODE_REPLACEMENT_CHARACTER_JSON_ENCODED . '"') . ']', $reportedTx->name);
+        $this->assertSame('test_TX_type', $reportedTx->type);
+    }
 }
diff --git a/tests/ElasticApmTests/UnitTests/Util/MockEventSink.php b/tests/ElasticApmTests/UnitTests/Util/MockEventSink.php
@@ -104,7 +104,6 @@ private function consumeMetadata(Metadata $original): void
         $deserialized = $this->validateAndDeserializeMetadata($serialized);
 
         self::assertValidMetadata($deserialized);
-        TestCaseBase::assertEqualsEx($original, $deserialized);
         $this->dataFromAgent->metadatas[] = $deserialized;
     }
 
@@ -116,7 +115,6 @@ private function consumeTransaction(Transaction $original): void
         $serialized = SerializationUtil::serializeAsJson($original);
         $deserialized = $this->validateAndDeserializeTransaction($serialized);
         $deserialized->assertValid();
-        $deserialized->assertEquals($original);
 
         TestCaseBase::assertNull($this->dataFromAgent->executionSegmentByIdOrNull($deserialized->id));
         ArrayUtilForTests::addUnique($deserialized->id, $deserialized, /* ref */ $this->dataFromAgent->idToTransaction);
@@ -135,7 +133,6 @@ private function consumeSpan(SpanToSendInterface $original): void
         $deserialized = $this->validateAndDeserializeSpan($serialized);
         $dbgCtx->add(['deserialized' => $deserialized]);
         $deserialized->assertValid();
-        $deserialized->assertEquals($original);
 
         TestCaseBase::assertNull($this->dataFromAgent->executionSegmentByIdOrNull($deserialized->id));
         ArrayUtilForTests::addUnique($deserialized->id, $deserialized, /* ref */ $this->dataFromAgent->idToSpan);
@@ -148,7 +145,6 @@ private function consumeError(Error $original): void
         $serialized = SerializationUtil::serializeAsJson($original);
         $deserialized = $this->validateAndDeserializeError($serialized);
         $deserialized->assertValid();
-        $deserialized->assertEquals($original);
 
         ArrayUtilForTests::addUnique($deserialized->id, $deserialized, /* ref */ $this->dataFromAgent->idToError);
     }
@@ -161,7 +157,6 @@ private function consumeMetricSet(MetricSet $original): void
         $serialized = SerializationUtil::serializeAsJson($original);
         $deserialized = $this->validateAndDeserializeMetricSet($serialized);
         MetricSetValidator::assertValid($deserialized);
-        TestCaseBase::assertEqualsEx($original, $deserialized);
 
         $this->dataFromAgent->metricSets[] = $deserialized;
     }

diff --git a/tests/ElasticApmTests/Util/AssertValidTrait.php b/tests/ElasticApmTests/Util/AssertValidTrait.php
@@ -25,7 +25,6 @@
 
 use Elastic\Apm\Impl\Constants;
 use Elastic\Apm\Impl\StackTraceFrame;
-use Elastic\Apm\Impl\Util\DbgUtil;
 use Elastic\Apm\Impl\Util\IdValidationUtil;
 use Elastic\Apm\Impl\Util\TextUtil;
 
@@ -293,47 +292,4 @@ public static function assertValidCount($count, int $minValue = 0): int
         TestCaseBase::assertGreaterThanOrEqual($minValue, $count);
         return $count;
     }
-
-    /**
-     * @param mixed $original
-     * @param mixed $dto
-     */
-    public static function assertEqualOriginalAndDto($original, $dto): void
-    {
-        AssertMessageStack::newScope(/* out */ $dbgCtx, AssertMessageStack::funcArgs());
-        $dbgCtx->add(['original type' => DbgUtil::getType($original), 'dto type' => DbgUtil::getType($dto)]);
-
-        if (is_object($dto)) {
-            TestCaseBase::assertIsObject($original);
-            $originalPropNameToVal = get_object_vars($original);
-            $dbgCtx->add(['originalPropNameToVal' => $originalPropNameToVal]);
-            $dbgCtx->pushSubScope();
-            foreach (get_object_vars($dto) as $dtoPropName => $dtoPropVal) {
-                $dbgCtx->clearCurrentSubScope(['dtoPropName' => $dtoPropName, 'dtoPropVal' => $dtoPropVal]);
-                if ($dtoPropVal !== null) {
-                    TestCaseBase::assertArrayHasKey($dtoPropName, $originalPropNameToVal);
-                }
-                if (array_key_exists($dtoPropName, $originalPropNameToVal)) {
-                    self::assertEqualOriginalAndDto($originalPropNameToVal[$dtoPropName], $dtoPropVal);
-                }
-            }
-            $dbgCtx->popSubScope();
-            return;
-        }
-
-        if (is_array($dto)) {
-            TestCaseBase::assertIsArray($original);
-            TestCaseBase::assertSameCount($original, $dto);
-            $dbgCtx->pushSubScope();
-            foreach ($dto as $dtoArrayKey => $dtoArrayVal) {
-                $dbgCtx->clearCurrentSubScope(['dtoArrayKey' => $dtoArrayKey, 'dtoArrayVal' => $dtoArrayVal]);
-                TestCaseBase::assertArrayHasKey($dtoArrayKey, $original);
-                self::assertEqualOriginalAndDto($original[$dtoArrayKey], $dtoArrayVal);
-            }
-            $dbgCtx->popSubScope();
-            return;
-        }
-
-        TestCaseBase::assertSame($original, $dto);
-    }
 }
diff --git a/tests/ElasticApmTests/Util/ErrorDto.php b/tests/ElasticApmTests/Util/ErrorDto.php
@@ -24,7 +24,6 @@
 namespace ElasticApmTests\Util;
 
 use Elastic\Apm\Impl\Constants;
-use Elastic\Apm\Impl\Error;
 use ElasticApmTests\Util\Deserialization\DeserializationUtil;
 
 class ErrorDto
@@ -152,9 +151,4 @@ public static function assertValidId($errorId): string
     {
         return self::assertValidIdEx($errorId, Constants::ERROR_ID_SIZE_IN_BYTES);
     }
-
-    public function assertEquals(Error $original): void
-    {
-        self::assertEqualOriginalAndDto($original, $this);
-    }
 }
diff --git a/tests/ElasticApmTests/Util/SpanDto.php b/tests/ElasticApmTests/Util/SpanDto.php
@@ -23,7 +23,6 @@
 
 namespace ElasticApmTests\Util;
 
-use Elastic\Apm\Impl\SpanToSendInterface;
 use Elastic\Apm\Impl\StackTraceFrame;
 use ElasticApmTests\Util\Deserialization\DeserializationUtil;
 use ElasticApmTests\Util\Deserialization\StacktraceDeserializer;
@@ -132,11 +131,6 @@ public function assertMatches(SpanExpectations $expectations): void
         SpanContextExpectations::assertNullableMatches($expectations->context, $this->context);
     }
 
-    public function assertEquals(SpanToSendInterface $original): void
-    {
-        self::assertEqualOriginalAndDto($original, $this);
-    }
-
     public function assertService(?string $targetType, ?string $targetName, string $destinationName, string $destinationResource, string $destinationType): void
     {
         TestCaseBase::assertNotNull($this->context);

diff --git a/tests/ElasticApmTests/Util/TransactionDto.php b/tests/ElasticApmTests/Util/TransactionDto.php
@@ -23,7 +23,6 @@
 
 namespace ElasticApmTests\Util;
 
-use Elastic\Apm\Impl\Transaction;
 use ElasticApmTests\Util\Deserialization\DeserializationUtil;
 
 class TransactionDto extends ExecutionSegmentDto
@@ -142,9 +141,4 @@ public function assertMatches(TransactionExpectations $expectations): void
             $this->context->assertValid();
         }
     }
-
-    public function assertEquals(Transaction $original): void
-    {
-        self::assertEqualOriginalAndDto($original, $this);
-    }
 }