Skip to content

Commit

Permalink
fix(json-serializer): fixes serialization for strings including backs…
Browse files Browse the repository at this point in the history
…lashes. (#189)

* fix(json-serializer): fixes serialization for strings including backslashes.

* chore(json-serializer): improves serialization.

* chore(lint): makes lint happy.
  • Loading branch information
jcchavezs authored Nov 24, 2020
1 parent 8776142 commit f06a857
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 14 deletions.
14 changes: 7 additions & 7 deletions src/Zipkin/Reporters/JsonV2Serializer.php
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public function serialize(array $spans): string

private static function serializeEndpoint(Endpoint $endpoint): string
{
$endpointStr = '{"serviceName":"' . \strtolower($endpoint->getServiceName()) . '"';
$endpointStr = '{"serviceName":"' . \strtolower(self::escapeString($endpoint->getServiceName())) . '"';

if ($endpoint->getIpv4() !== null) {
$endpointStr .= ',"ipv4":"' . $endpoint->getIpv4() . '"';
Expand All @@ -50,10 +50,10 @@ private static function serializeEndpoint(Endpoint $endpoint): string
return $endpointStr . '}';
}

private static function escapeQuotes(string $s): string
private static function escapeString(string $s): string
{
$encodedString = json_encode($s);
return $encodedString ? trim($encodedString, '"') : $s;
$encodedString = \json_encode($s);
return $encodedString === false ? $s : \mb_substr($encodedString, 1, -1);
}

private function serializeSpan(ReadbackSpan $span): string
Expand All @@ -64,7 +64,7 @@ private function serializeSpan(ReadbackSpan $span): string
. ',"timestamp":' . $span->getTimestamp();

if ($span->getName() !== null) {
$spanStr .= ',"name":"' . \strtolower($span->getName()) . '"';
$spanStr .= ',"name":"' . \strtolower(self::escapeString($span->getName())) . '"';
}

if ($span->getDuration() !== null) {
Expand Down Expand Up @@ -104,7 +104,7 @@ private function serializeSpan(ReadbackSpan $span): string
} else {
$spanStr .= ',';
}
$spanStr .= '{"value":"' . self::escapeQuotes($annotation['value'])
$spanStr .= '{"value":"' . self::escapeString($annotation['value'])
. '","timestamp":' . $annotation['timestamp'] . '}';
}
$spanStr .= ']';
Expand All @@ -125,7 +125,7 @@ private function serializeSpan(ReadbackSpan $span): string
} else {
$spanStr .= ',';
}
$spanStr .= '"' . $key . '":"' . self::escapeQuotes($value) . '"';
$spanStr .= '"' . $key . '":"' . self::escapeString($value) . '"';
}
$spanStr .= '}';
}
Expand Down
6 changes: 3 additions & 3 deletions tests/Integration/Reporters/Http/CurlFactoryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ function (RequestInterface $request, ResponseInterface &$response) use ($t) {
'endpoint_url' => $server->getUrl(),
]);

$curlClient(json_encode([]));
$curlClient(\json_encode([]));
$this->assertTrue(true);
} finally {
$server->stop();
Expand Down Expand Up @@ -69,7 +69,7 @@ function (RequestInterface $request, ResponseInterface &$response) use ($t) {
'headers' => ['From' => '[email protected]', 'Content-Type' => 'test'],
]);

$curlClient(json_encode([]));
$curlClient(\json_encode([]));
$this->assertTrue(true);
} finally {
$server->stop();
Expand Down Expand Up @@ -137,7 +137,7 @@ function (RequestInterface $request, ResponseInterface &$response) use ($t) {
]);

ob_start();
$curlClient(json_encode([]));
$curlClient(\json_encode([]));
} finally {
$server->stop();
$output = ob_get_clean();
Expand Down
8 changes: 4 additions & 4 deletions tests/Unit/Reporters/JsonV2SerializerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ public function testErrorTagIsNotClobberedBySpanError()
$this->assertEquals($expectedSerialization, $serializedSpans);
}

public function testJSONTagsAreSerializedCorrectly()
public function testStringValuesAreEscapedAndSerializedCorrectly()
{
$jsonValue = '{"name":"Kurt"}';
$mutilineValue = <<<EOD
Expand All @@ -75,16 +75,16 @@ public function testJSONTagsAreSerializedCorrectly()
$span = Span::createFromContext($context, $localEndpoint);
$startTime = 1594044779509687;
$span->start($startTime);
$span->setName('Test');
$span->setName('My\Command');
$span->tag('test_key_1', $jsonValue);
$span->tag('test_key_2', $mutilineValue);
$span->finish($startTime + 1000);
$serializer = new JsonV2Serializer();
$serializedSpans = $serializer->serialize([$span]);

$expectedSerialization = '[{'
. '"id":"186f11b67460db4e","traceId":"186f11b67460db4e","timestamp":1594044779509687,"name":"test",'
. '"duration":1000,"localEndpoint":{"serviceName":"service1"},'
. '"id":"186f11b67460db4e","traceId":"186f11b67460db4e","timestamp":1594044779509687,'
. '"name":"my\\\\command","duration":1000,"localEndpoint":{"serviceName":"service1"},'
. '"tags":{"test_key_1":"{\"name\":\"Kurt\"}","test_key_2":"foo\nbar"}'
. '}]';

Expand Down

0 comments on commit f06a857

Please sign in to comment.