-
Notifications
You must be signed in to change notification settings - Fork 4
fix: cap payload depth and improve serialized arg readability for Hawk #74
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
base: master
Are you sure you want to change the base?
Changes from all commits
47ffb27
e5844fa
058dfaa
f942630
65d6ab5
687d446
fb2293b
7991027
358da18
e832e06
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -40,6 +40,12 @@ class EventPayloadBuilder | |
| 'additionalData', | ||
| ]; | ||
|
|
||
| /** | ||
| * {@see transformForJsonRecursive} max nesting — stops runaway recursion; combined with array reference | ||
| * stack for the same circular detection as {@see Serializer::prepare()}. | ||
| */ | ||
| private const TRANSFORM_JSON_MAX_DEPTH = 32; | ||
|
|
||
| /** | ||
| * EventPayloadFactory constructor. | ||
| */ | ||
|
|
@@ -158,9 +164,15 @@ private function normalizeBacktrace(array $stack): array | |
| $functionName = (string) $frame['functionName']; | ||
| } | ||
|
|
||
| $arguments = $this->buildArgumentsList($frame); | ||
|
|
||
| $additional = []; | ||
| foreach ($frame as $key => $value) { | ||
| if (!in_array($key, self::ALLOWED_KEYS, true)) { | ||
| // Mapped to `arguments` via StacktraceFrameBuilder / string list; do not dump raw `args` here | ||
| if ($key === 'args') { | ||
| continue; | ||
| } | ||
| // Drop heavy/unserializable objects from 'object' field; store class name instead | ||
| if ($key === 'object') { | ||
| $value = is_object($value) ? get_class($value) : $value; | ||
|
|
@@ -176,9 +188,7 @@ private function normalizeBacktrace(array $stack): array | |
| 'column' => null, | ||
| 'sourceCode' => isset($frame['sourceCode']) && is_array($frame['sourceCode']) ? $frame['sourceCode'] : null, | ||
| 'function' => $functionName, | ||
| // Keep arguments only if it already looks like desired string[]; otherwise omit | ||
| // Limit argument processing to first 10 items to avoid performance issues | ||
| 'arguments' => (isset($frame['arguments']) && is_array($frame['arguments'])) ? array_values(array_map('strval', array_slice($frame['arguments'], 0, 10))) : [], | ||
| 'arguments' => $arguments, | ||
| 'additionalData'=> $additional, | ||
| ]); | ||
| } | ||
|
|
@@ -223,19 +233,78 @@ private function sanitizeArrayKeys($value) | |
| } | ||
|
|
||
| /** | ||
| * Transform values to JSON-serializable representation | ||
| * Build Hawk `arguments`: string list like "name = serializedValue" (from raw `args` via Serializer). | ||
| * Limits the number of lines ({@see StacktraceFrameBuilder::MAX_FRAME_ARGUMENTS}). Serialized values are not | ||
| * length-truncated; only param names are capped ({@see StacktraceFrameBuilder::formatTruncatedArgumentLine}); | ||
| * prebuilt strings are split on the first `" = "` with {@see StacktraceFrameBuilder::truncatePrebuiltArgumentLine}. | ||
| * | ||
| * @param array $frame | ||
| * | ||
| * @return array | ||
| */ | ||
| private function buildArgumentsList(array $frame): array | ||
| { | ||
| $max = StacktraceFrameBuilder::MAX_FRAME_ARGUMENTS; | ||
|
|
||
| if (isset($frame['arguments']) && is_array($frame['arguments'])) { | ||
| $out = []; | ||
| foreach (array_slice($frame['arguments'], 0, $max) as $line) { | ||
| $out[] = StacktraceFrameBuilder::truncatePrebuiltArgumentLine((string) $line); | ||
| } | ||
|
|
||
| return $out; | ||
| } | ||
|
|
||
| if (!empty($frame['args']) && is_array($frame['args'])) { | ||
| $out = []; | ||
| foreach (array_slice($this->stacktraceFrameBuilder->getFormattedArguments($frame), 0, $max) as $line) { | ||
| $out[] = (string) $line; | ||
| } | ||
|
|
||
| return $out; | ||
| } | ||
|
|
||
| return []; | ||
| } | ||
|
|
||
| /** | ||
| * Transform frame extra fields for JSON — scalars, shallow objects, arrays with depth/cycle limits. | ||
| * | ||
| * @param mixed $value | ||
| * | ||
| * @return mixed | ||
| */ | ||
| private function transformForJson($value) | ||
| { | ||
| $stack = []; | ||
|
|
||
| return $this->transformForJsonRecursive($value, 0, $stack); | ||
| } | ||
|
|
||
| /** | ||
| * @param mixed $value | ||
| * | ||
| * @return mixed | ||
| */ | ||
| private function transformForJsonRecursive($value, int $depth, array &$stack) | ||
| { | ||
| if ($depth > self::TRANSFORM_JSON_MAX_DEPTH) { | ||
| return '[max depth]'; | ||
| } | ||
|
|
||
| if (is_array($value)) { | ||
| foreach ($stack as $ancestor) { | ||
| if ($value === $ancestor) { | ||
| return '[circular]'; | ||
| } | ||
| } | ||
|
|
||
| $stack[] = $value; | ||
| $result = []; | ||
| foreach ($value as $k => $v) { | ||
| $result[$k] = $this->transformForJson($v); | ||
| $result[$k] = $this->transformForJsonRecursive($v, $depth + 1, $stack); | ||
| } | ||
|
Comment on lines
295
to
306
|
||
| array_pop($stack); | ||
|
|
||
| return $result; | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -14,6 +14,17 @@ | |||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||
| final class StacktraceFrameBuilder | ||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||
| * Max function arguments to include per frame (payload size, CPU, Hawk limits). | ||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||
| public const MAX_FRAME_ARGUMENTS = 20; | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||
| * Max UTF-8 byte length for the argument name (left-hand side only). | ||||||||||||||||||||||||||||||||||
| * Serialized values from {@see Serializer::serializeValue()} are not length-capped so JSON stays intact. | ||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||
| public const MAX_ARGUMENT_NAME_BYTES = 256; | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||
| * @var Serializer | ||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||
|
|
@@ -183,6 +194,19 @@ private function composeFunctionName(array $frame): string | |||||||||||||||||||||||||||||||||
| return $functionName; | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||
| * Format `args` from a raw debug_backtrace() frame to Hawk `arguments` (list of "name = value" strings). | ||||||||||||||||||||||||||||||||||
| * Public so {@see EventPayloadBuilder} can map `args` without duplicating logic. | ||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||
| * @param array $frame | ||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||
| * @return array | ||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||
| public function getFormattedArguments(array $frame): array | ||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||
| return $this->getArgs($frame); | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||
| * Get function arguments for a frame | ||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||
|
|
@@ -216,6 +240,9 @@ private function getArgs(array $frame): array | |||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||
| if (!$reflection) { | ||||||||||||||||||||||||||||||||||
| foreach ($frame['args'] as $index => $value) { | ||||||||||||||||||||||||||||||||||
| if ($index >= self::MAX_FRAME_ARGUMENTS) { | ||||||||||||||||||||||||||||||||||
| break; | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
| $arguments['arg' . $index] = $value; | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||
|
|
@@ -231,6 +258,10 @@ private function getArgs(array $frame): array | |||||||||||||||||||||||||||||||||
| $paramName = $reflectionParam->getName(); | ||||||||||||||||||||||||||||||||||
| $paramPosition = $reflectionParam->getPosition(); | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| if ($paramPosition >= self::MAX_FRAME_ARGUMENTS) { | ||||||||||||||||||||||||||||||||||
| break; | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| if (isset($frame['args'][$paramPosition])) { | ||||||||||||||||||||||||||||||||||
| $arguments[$paramName] = $frame['args'][$paramPosition]; | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
@@ -243,18 +274,91 @@ private function getArgs(array $frame): array | |||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||
| $newArguments = []; | ||||||||||||||||||||||||||||||||||
| foreach ($arguments as $name => $value) { | ||||||||||||||||||||||||||||||||||
| $value = $this->serializer->serializeValue($value); | ||||||||||||||||||||||||||||||||||
| $serialized = $this->serializer->serializeValue($value); | ||||||||||||||||||||||||||||||||||
| $newArguments[] = self::formatTruncatedArgumentLine((string) $name, $serialized); | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||
| $newArguments[] = sprintf('%s = %s', $name, $value); | ||||||||||||||||||||||||||||||||||
| } catch (\Exception $e) { | ||||||||||||||||||||||||||||||||||
| // Ignore unknown types | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
| return $newArguments; | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||
| * Build `"name = value"` — only the name may be shortened; serialized value is kept whole (valid JSON, etc.). | ||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||
| public static function formatTruncatedArgumentLine(string $name, string $serializedValue): string | ||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||
| $namePart = self::truncateUtf8StringToMaxBytes($name, self::MAX_ARGUMENT_NAME_BYTES); | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| return $namePart . ' = ' . $serializedValue; | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+287
to
+292
|
||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||
| * Normalize one prebuilt `name = serializedValue` line: split at the first `" = "`, cap name only; value unchanged. | ||||||||||||||||||||||||||||||||||
| * Lines without `" = "` are returned as-is (no length limit). | ||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||
| public static function truncatePrebuiltArgumentLine(string $line): string | ||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||
| $separator = ' = '; | ||||||||||||||||||||||||||||||||||
| $position = strpos($line, $separator); | ||||||||||||||||||||||||||||||||||
| if ($position === false) { | ||||||||||||||||||||||||||||||||||
| return $line; | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+296
to
+303
|
||||||||||||||||||||||||||||||||||
| * Lines without `" = "` are returned as-is (no length limit). | |
| */ | |
| public static function truncatePrebuiltArgumentLine(string $line): string | |
| { | |
| $separator = ' = '; | |
| $position = strpos($line, $separator); | |
| if ($position === false) { | |
| return $line; | |
| * Lines without `" = "` are truncated as whole lines to the configured byte cap. | |
| */ | |
| public static function truncatePrebuiltArgumentLine(string $line): string | |
| { | |
| $separator = ' = '; | |
| $position = strpos($line, $separator); | |
| if ($position === false) { | |
| return self::truncateUtf8StringToMaxBytes($line, self::MAX_ARGUMENT_NAME_BYTES); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PR description mentions capping/truncating each stack argument line by byte length, but
buildArgumentsList()explicitly keeps serialized values unbounded and the tests preserve very long prebuilt argument lines. This can still produce oversized events. Either implement a per-line byte cap (for both prebuiltargumentsand formattedargs) or update the PR description/contract expectations to match the current behavior.