Skip to content
This repository was archived by the owner on Oct 3, 2023. It is now read-only.

Commit 1cc9895

Browse files
chingor13chingor13
authored
Extension: Improve input type handling (#186)
* Add tests for span id, kind, name types * Fix string handling for span name, id, and kind * Fix warning message expectation for php 7.0 * Add tests for verifying types of same process, stacktrace and starttime fields * Fix span start time handling * Fix memory leak when specifying stackTrace * Add test for invalid stacktrace input * Trigger warning for non-array input for stackTrace * Name attribute cannot be null * Rename CastableObject -> UncastableObject in uncastable test
1 parent f29bbf7 commit 1cc9895

22 files changed

+372
-26
lines changed

‎ext/opencensus_trace.c‎

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,8 @@
2727
#include "Zend/zend_exceptions.h"
2828
#include "zend_extensions.h"
2929
#include "standard/php_math.h"
30-
#include "ext/standard/info.h"
3130
#include "standard/php_rand.h"
31+
#include "ext/standard/info.h"
3232

3333
/**
3434
* True globals for storing the original zend_execute_ex and
@@ -136,9 +136,14 @@ PHP_FUNCTION(opencensus_version)
136136
RETURN_STRING(PHP_OPENCENSUS_VERSION);
137137
}
138138

139+
/**
140+
* Fetch the spanId zend_string value from the provided array.
141+
* Note that the returned zend_string must be released by the caller.
142+
*/
139143
static zend_string *span_id_from_options(HashTable *options)
140144
{
141145
zval *val;
146+
zend_string *str = NULL;
142147
if (options == NULL) {
143148
return NULL;
144149
}
@@ -148,9 +153,24 @@ static zend_string *span_id_from_options(HashTable *options)
148153
return NULL;
149154
}
150155

151-
return Z_STR_P(val);
156+
switch (Z_TYPE_P(val)) {
157+
case IS_STRING:
158+
str = zval_get_string(val);
159+
break;
160+
case IS_LONG:
161+
str = _php_math_longtobase(val, 16);
162+
break;
163+
}
164+
165+
if (str == NULL) {
166+
php_error_docref(NULL, E_WARNING, "Provided spanId should be a hex string");
167+
return NULL;
168+
}
169+
170+
return str;
152171
}
153172

173+
/*Fetch the span struct for the spanId from the provided array. */
154174
static opencensus_trace_span_t *span_from_options(zval *options)
155175
{
156176
zend_string *span_id = NULL;
@@ -163,6 +183,7 @@ static opencensus_trace_span_t *span_from_options(zval *options)
163183
span_id = span_id_from_options(Z_ARR_P(options));
164184
if (span_id != NULL) {
165185
span = (opencensus_trace_span_t *)zend_hash_find_ptr(OPENCENSUS_TRACE_G(spans), span_id);
186+
zend_string_release(span_id);
166187
}
167188

168189
return span;
@@ -384,7 +405,7 @@ static opencensus_trace_span_t *opencensus_trace_begin(zend_string *name, zend_e
384405

385406
span->start = opencensus_now();
386407
span->name = zend_string_copy(name);
387-
if (span_id) {
408+
if (span_id != NULL) {
388409
span->span_id = zend_string_copy(span_id);
389410
} else {
390411
span->span_id = generate_span_id();
@@ -479,6 +500,9 @@ PHP_FUNCTION(opencensus_trace_begin)
479500
} else {
480501
span_id = span_id_from_options(Z_ARR_P(span_options));
481502
span = opencensus_trace_begin(function_name, execute_data, span_id TSRMLS_CC);
503+
if (span_id != NULL) {
504+
zend_string_release(span_id);
505+
}
482506
opencensus_trace_span_apply_span_options(span, span_options);
483507
}
484508

‎ext/opencensus_trace_span.c‎

Lines changed: 29 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -553,34 +553,45 @@ int opencensus_trace_span_apply_span_options(opencensus_trace_span_t *span, zval
553553
zend_hash_merge(span->attributes, Z_ARRVAL_P(v), zval_add_ref, 0);
554554
} else if (strcmp(ZSTR_VAL(k), "startTime") == 0) {
555555
switch (Z_TYPE_P(v)) {
556-
case IS_DOUBLE:
557-
span->start = Z_DVAL_P(v);
556+
case IS_NULL:
558557
break;
559558
case IS_LONG:
560-
span->start = (double) Z_LVAL_P(v);
559+
case IS_DOUBLE:
560+
span->start = zval_get_double(v);
561+
break;
562+
default:
563+
php_error_docref(NULL, E_WARNING, "Provided startTime should be a float");
561564
break;
562565
}
563566
} else if (strcmp(ZSTR_VAL(k), "name") == 0) {
564-
if (span->name) {
565-
zend_string_release(span->name);
567+
if (!Z_ISNULL_P(v)) {
568+
if (span->name) {
569+
zend_string_release(span->name);
570+
}
571+
span->name = zval_get_string(v);
572+
} else {
573+
php_error_docref(NULL, E_WARNING, "Provided name should be a string");
566574
}
567-
span->name = zend_string_copy(Z_STR_P(v));
568-
} else if (strcmp(ZSTR_VAL(k), "spanId") == 0) {
569-
if (span->span_id) {
570-
zend_string_release(span->span_id);
571-
}
572-
span->span_id = zend_string_copy(Z_STR_P(v));
573575
} else if (strcmp(ZSTR_VAL(k), "kind") == 0) {
574-
if (span->kind) {
575-
zend_string_release(span->kind);
576+
if (Z_TYPE_P(v) == IS_STRING) {
577+
if (span->kind) {
578+
zend_string_release(span->kind);
579+
}
580+
span->kind = zval_get_string(v);
581+
} else {
582+
php_error_docref(NULL, E_WARNING, "Provided kind should be a string");
576583
}
577-
span->kind = zend_string_copy(Z_STR_P(v));
578584
} else if (strcmp(ZSTR_VAL(k), "sameProcessAsParentSpan") == 0) {
579-
if (Z_TYPE_P(v) == IS_FALSE) {
580-
span->same_process_as_parent_span = 0;
581-
}
585+
span->same_process_as_parent_span = zend_is_true(v);
582586
} else if (strcmp(ZSTR_VAL(k), "stackTrace") == 0) {
583-
ZVAL_COPY(&span->stackTrace, v);
587+
if (Z_TYPE_P(v) == IS_ARRAY) {
588+
if (!Z_ISNULL(span->stackTrace)) {
589+
ZVAL_DESTRUCTOR(&span->stackTrace);
590+
}
591+
ZVAL_COPY(&span->stackTrace, v);
592+
} else {
593+
php_error_docref(NULL, E_WARNING, "Provided stackTrace should be an array");
594+
}
584595
}
585596
} ZEND_HASH_FOREACH_END();
586597
return SUCCESS;

‎ext/tests/name_castable_object.phpt‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
--TEST--
2+
OpenCensus Trace: Providing integer as name
3+
--FILE--
4+
<?php
5+
6+
class CastableObject
7+
{
8+
private $value = 'my value';
9+
10+
public function __toString()
11+
{
12+
return $this->value;
13+
}
14+
}
15+
16+
$obj = new CastableObject();
17+
opencensus_trace_begin('foo', ['name' => $obj]);
18+
opencensus_trace_finish();
19+
$spans = opencensus_trace_list();
20+
21+
echo 'Number of spans: ' . count($spans) . PHP_EOL;
22+
$span = $spans[0];
23+
var_dump($span->name());
24+
25+
?>
26+
--EXPECTF--
27+
Number of spans: 1
28+
string(8) "my value"

‎ext/tests/name_double.phpt‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
--TEST--
2+
OpenCensus Trace: Providing integer as name
3+
--FILE--
4+
<?php
5+
6+
opencensus_trace_begin('foo', ['name' => 1.23]);
7+
opencensus_trace_finish();
8+
$spans = opencensus_trace_list();
9+
10+
echo 'Number of spans: ' . count($spans) . PHP_EOL;
11+
$span = $spans[0];
12+
var_dump($span->name());
13+
14+
?>
15+
--EXPECTF--
16+
Number of spans: 1
17+
string(4) "1.23"

‎ext/tests/name_integer.phpt‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
--TEST--
2+
OpenCensus Trace: Providing integer as name
3+
--FILE--
4+
<?php
5+
6+
opencensus_trace_begin('foo', ['name' => 123]);
7+
opencensus_trace_finish();
8+
$spans = opencensus_trace_list();
9+
10+
echo 'Number of spans: ' . count($spans) . PHP_EOL;
11+
$span = $spans[0];
12+
var_dump($span->name());
13+
14+
?>
15+
--EXPECTF--
16+
Number of spans: 1
17+
string(3) "123"

‎ext/tests/name_null.phpt‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
--TEST--
2+
OpenCensus Trace: Providing integer as name
3+
--FILE--
4+
<?php
5+
6+
opencensus_trace_begin('foo', ['name' => null]);
7+
opencensus_trace_finish();
8+
$spans = opencensus_trace_list();
9+
10+
echo 'Number of spans: ' . count($spans) . PHP_EOL;
11+
$span = $spans[0];
12+
var_dump($span->name());
13+
14+
?>
15+
--EXPECTF--
16+
Warning: opencensus_trace_begin(): Provided name should be a string in %s on line %d
17+
Number of spans: 1
18+
string(3) "foo"

‎ext/tests/name_uncastable_object.phpt‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
--TEST--
2+
OpenCensus Trace: Providing integer as name
3+
--FILE--
4+
<?php
5+
6+
class UncastableObject
7+
{
8+
private $value = 'my value';
9+
}
10+
11+
$obj = new UncastableObject();
12+
opencensus_trace_begin('foo', ['name' => $obj]);
13+
opencensus_trace_finish();
14+
$spans = opencensus_trace_list();
15+
16+
echo 'Number of spans: ' . count($spans) . PHP_EOL;
17+
$span = $spans[0];
18+
var_dump($span->name());
19+
20+
?>
21+
--EXPECTF--
22+
%s fatal error: Object of class UncastableObject could not be converted to string in %s/name_uncastable_object.php on line %d

‎ext/tests/span_id_double.phpt‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
--TEST--
2+
OpenCensus Trace: Test setting span id to null
3+
--FILE--
4+
<?php
5+
6+
opencensus_trace_begin('root', ['spanId' => 1.23]);
7+
opencensus_trace_finish();
8+
9+
$traces = opencensus_trace_list();
10+
echo "Number of traces: " . count($traces) . "\n";
11+
$span = $traces[0];
12+
var_dump($span->spanId());
13+
?>
14+
--EXPECTF--
15+
Warning: opencensus_trace_begin(): Provided spanId should be a hex string in %s on line %d
16+
Number of traces: 1
17+
string(%d) "%s"

‎ext/tests/span_id_integer.phpt‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
--TEST--
2+
OpenCensus Trace: Test setting span id to null
3+
--FILE--
4+
<?php
5+
6+
opencensus_trace_begin('root', ['spanId' => 123]);
7+
opencensus_trace_finish();
8+
9+
$traces = opencensus_trace_list();
10+
echo "Number of traces: " . count($traces) . "\n";
11+
$span = $traces[0];
12+
var_dump($span->spanId());
13+
?>
14+
--EXPECT--
15+
Number of traces: 1
16+
string(2) "7b"

‎ext/tests/span_id_null.phpt‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
--TEST--
2+
OpenCensus Trace: Test setting span id to null
3+
--FILE--
4+
<?php
5+
6+
opencensus_trace_begin('root', ['spanId' => null]);
7+
opencensus_trace_finish();
8+
9+
$traces = opencensus_trace_list();
10+
echo "Number of traces: " . count($traces) . "\n";
11+
$span = $traces[0];
12+
var_dump($span->spanId());
13+
?>
14+
--EXPECTF--
15+
Warning: opencensus_trace_begin(): Provided spanId should be a hex string in %s on line %d
16+
Number of traces: 1
17+
string(%d) "%s"

0 commit comments

Comments
 (0)