raythang commented 3 years ago

Describe your environment

php (fpm-fcgi) 7.3.21
nginx 1.15.6

Steps to reproduce

<?php
use OpenTelemetry\Sdk\Trace\PropagationMap;
use OpenTelemetry\Sdk\Trace\TraceContext;

$carrier = getallheaders();
$map = new PropagationMap();

$context = TraceContext::extract($carrier, $map);  // <--- this call induces the problem

What is the expected behavior?

No TypeError is raised, and the context is successfully extracted.

What is the actual behavior?

The extract call above induces the following:

Uncaught TypeError: strtolower() expects parameter 1 to be string,
int given in /vendor/open-telemetry/opentelemetry/sdk/Trace/PropagationMap.php:28

Additional context

getallheaders returns an associative array of request headers (name => value) https://www.php.net/manual/en/function.getallheaders.php

However, it appears that getallheaders may return integer keys for header names. Consider:

[rthang@rthang.dev current]$ cat echo-getallheaders.php
<?php

echo var_export(getallheaders(), true);
echo PHP_EOL;

[rthang@rthang.dev current]$ curl localhost/echo-getallheaders.php -H '1: numeric-headers-for-the-loss'
array (
  1 => 'numeric-headers-for-the-loss',
  'Accept' => '*/*',
  'Host' => 'localhost',
  'User-Agent' => 'curl/7.29.0',
  'Content-Type' => 'application/x-www-form-urlencoded',
  'X-Forwarded-For' => '127.0.0.1',
  'Content-Length' => '',
)

This conflicts with the assumption by PropagationMap that the $carrier's keys are all strings, triggering the TypeError from strtolower that we see today. https://github.com/open-telemetry/opentelemetry-php/blob/4c0da3dec054d6fd0d4b570ef1e5f11fe2d7fd88/sdk/Trace/PropagationMap.php#L28

Fahmy-Mohammed commented 3 years ago

Hi @raythang,

Thanks for highlighting this error. I will work on a fix for this today.

raythang commented 3 years ago

Thanks @Fahmy-Mohammed lmk if you need more detail.