Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MongoDBFormatter can affect passed by reference arguments when writing trace to log #1237

Open
FloatingMaster opened this issue Dec 7, 2018 · 5 comments
Labels
Milestone

Comments

@FloatingMaster
Copy link

Steps to reproduce
Error handler:

public function handleError($type, $message, $file, $line) 
{
  $exception = new \ErrorException();
  $trace = $exception->getTrace();
  array_shift($trace);

  $this->getLogger()->log($this->errorTypeToLogLevel($type), $message, $trace)
}

Next in MongoDBFormatter::formatArray we have

foreach ($record as $name => $value) {
  ...
  } elseif (is_object($value)) {
    $record[$name] = $this->formatObject($value, $nestingLevel + 1);
  }
}

If there was any pass-by-reference arguments (e.g. foo(MyObj &$obj)) the object by reference will be replaced by formatted array.

Example:

public function testFoo()
{
  $a = new \stdClass();
    	
  var_dump($a);
  $this->foo($a);

  var_dump($a);

 }

public function foo(\stdClass &$a)
{
  $b = [];
  $b = $b['non-existing'];
}

Output:

class stdClass#134 (0) {
}
array(1) {
  'class' =>
  string(8) "stdClass"
}

@Seldaek Seldaek added the Bug label Dec 11, 2018
@Seldaek Seldaek added this to the 1.x milestone Dec 11, 2018
@Seldaek
Copy link
Owner

Seldaek commented Dec 11, 2018

This probably should be fixed (or at least checked if they are affected) for all normalizers.

@juan-morales
Copy link
Contributor

But, in such a case, what is adviced to do here in order to "fix" this "issue"?

As far as I know , there is no reliable way to check if a variable is a reference in PHP, and "cloning just in case" is not an option.

Am I missing something here?

@Seldaek
Copy link
Owner

Seldaek commented Oct 22, 2021

It might just be about assigning things to a new array instead of the same to make sure we copy the stack trace and get rid of references.

I'm not sure either.. Doesn't seem like a super critical issue tbh, it's so unlikely to happen.

@FloatingMaster
Copy link
Author

We solved the issue by recursivly converting record to array of simple type fields. Objects are converted to array through bsonSerialize, jsonSerialize or reflections

@juan-morales
Copy link
Contributor

@FloatingMaster you applied that code on private code? Can we see it so we can better understand what was suppose to be done?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants