Skip to content

Commit

Permalink
Improve argument handling in serialization
Browse files Browse the repository at this point in the history
- Ensure references do not get modified
- Standardize truncation logic
  • Loading branch information
dcramer committed Jan 25, 2017
1 parent b6f94b4 commit cd04e76
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 38 deletions.
36 changes: 24 additions & 12 deletions lib/Raven/Stacktrace.php
Original file line number Diff line number Diff line change
Expand Up @@ -122,10 +122,7 @@ public static function get_default_context($frame, $frame_arg_limit = Raven_Clie
$i = 1;
$args = array();
foreach ($frame['args'] as $arg) {
if (is_string($arg) || is_numeric($arg)) {
$arg = substr($arg, 0, $frame_arg_limit);
}
$args['param'.$i] = $arg;
$args['param'.$i] = self::serialize_argument($arg, $frame_arg_limit);
$i++;
}
return $args;
Expand Down Expand Up @@ -157,7 +154,9 @@ public static function get_frame_context($frame, $frame_arg_limit = Raven_Client
return array();
} else {
// Sanitize the file path
return array('param1' => $frame['args'][0]);
return array(
'param1' => self::serialize_argument($frame['args'][0], $frame_arg_limit),
);
}
}
try {
Expand All @@ -180,15 +179,9 @@ public static function get_frame_context($frame, $frame_arg_limit = Raven_Client

$args = array();
foreach ($frame['args'] as $i => $arg) {
$arg = self::serialize_argument($arg, $frame_arg_limit);
if (isset($params[$i])) {
// Assign the argument by the parameter name
if (is_array($arg)) {
foreach ($arg as $key => $value) {
if (is_string($value) || is_numeric($value)) {
$arg[$key] = substr($value, 0, $frame_arg_limit);
}
}
}
$args[$params[$i]->name] = $arg;
} else {
$args['param'.$i] = $arg;
Expand All @@ -198,6 +191,25 @@ public static function get_frame_context($frame, $frame_arg_limit = Raven_Client
return $args;
}

private static function serialize_argument($arg, $frame_arg_limit)
{
if (is_array($arg)) {
$_arg = array();
foreach ($arg as $key => $value) {
if (is_string($value) || is_numeric($value)) {
$_arg[$key] = substr($value, 0, $frame_arg_limit);
} else {
$_arg[$key] = $value;
}
}
return $_arg;
} elseif (is_string($arg) || is_numeric($arg)) {
return substr($arg, 0, $frame_arg_limit);
} else {
return $arg;
}
}

private static function strip_prefixes($filename, $prefixes)
{
if ($prefixes === null) {
Expand Down
45 changes: 19 additions & 26 deletions test/Raven/Tests/StacktraceTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -71,41 +71,34 @@ public function testSimpleTrace()

public function testDoesNotModifyCaptureVars()
{
$stack = array(
array(
"file" => dirname(__FILE__) . "/resources/a.php",
"line" => 11,
"function" => "a_test",
"args"=> array(
"friend",
),
),
array(
"file" => dirname(__FILE__) . "/resources/b.php",
"line" => 3,
"args"=> array(
"/tmp/a.php",
),
"function" => "include_once",
),
);

// PHP's errcontext as passed to the error handler contains REFERENCES to any vars that were in the global scope.
// Modification of these would be really bad, since if control is returned (non-fatal error) we'll have altered the state of things!
$originalFoo = "bloopblarp";
$iAmFoo = $originalFoo;
$vars = array(
"foo" => &$iAmFoo
$originalFoo = 'bloopblarp';
$newFoo = $originalFoo;
$nestedArray = array(
'key' => 'xxxxxxxxxx',
);

$frames = Raven_Stacktrace::get_stack_info($stack, true, $vars, 5);
$frame = array(
"file" => dirname(__FILE__) . "/resources/a.php",
"line" => 9,
"args"=> array(
&$newFoo,
&$nestedArray,
),
"function" => "a_test",
);

$result = Raven_Stacktrace::get_frame_context($frame, 5);

// Check we haven't modified our vars.
$this->assertEquals($originalFoo, $vars['foo']);
$this->assertEquals($originalFoo, 'bloopblarp');
$this->assertEquals($nestedArray['key'], 'xxxxxxxxxx');

$frame = $frames[1];
// Check that we did truncate the variable in our output
$this->assertEquals(strlen($frame['vars']['foo']), 5);
$this->assertEquals($result['param1'], 'bloop');
$this->assertEquals($result['param2']['key'], 'xxxxx');
}

public function testDoesFixFrameInfo()
Expand Down

0 comments on commit cd04e76

Please sign in to comment.