-
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
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,11 @@ class EventPayloadBuilder | |
| 'additionalData', | ||
| ]; | ||
|
|
||
| /** | ||
| * {@see transformForJson} max nesting — protects against $GLOBALS self-reference and similar cycles. | ||
| */ | ||
| private const TRANSFORM_JSON_MAX_DEPTH = 32; | ||
|
|
||
| /** | ||
| * EventPayloadFactory constructor. | ||
| */ | ||
|
|
@@ -158,15 +163,21 @@ 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; | ||
| } | ||
|
|
||
| $additional[$key] = $this->transformForJson($value); | ||
| $additional[$key] = $this->transformForJson($value, 0); | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -176,9 +187,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, | ||
| ]); | ||
| } | ||
|
|
@@ -222,19 +231,59 @@ private function sanitizeArrayKeys($value) | |
| return $sanitized; | ||
| } | ||
|
|
||
| /** | ||
| * 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 values to JSON-serializable representation | ||
| * | ||
| * @param mixed $value | ||
| * @param int $depth | ||
| * | ||
| * @return mixed | ||
| */ | ||
| private function transformForJson($value) | ||
| private function transformForJson($value, int $depth = 0) | ||
| { | ||
| if ($depth > self::TRANSFORM_JSON_MAX_DEPTH) { | ||
| return '[max depth]'; | ||
| } | ||
|
|
||
| if (is_array($value)) { | ||
| $result = []; | ||
| foreach ($value as $k => $v) { | ||
| $result[$k] = $this->transformForJson($v); | ||
| $result[$k] = $this->transformForJson($v, $depth + 1); | ||
| } | ||
|
Comment on lines
283
to
287
|
||
|
|
||
| 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]; | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
@@ -246,15 +277,70 @@ private function getArgs(array $frame): array | |||||||||||||||||||||||||||||||||
| $value = $this->serializer->serializeValue($value); | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||
| $newArguments[] = sprintf('%s = %s', $name, $value); | ||||||||||||||||||||||||||||||||||
| $newArguments[] = self::formatTruncatedArgumentLine((string) $name, $value); | ||||||||||||||||||||||||||||||||||
| } catch (\Exception $e) { | ||||||||||||||||||||||||||||||||||
| // Ignore unknown types | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
Comment on lines
279
to
283
|
||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| $arguments = $newArguments; | ||||||||||||||||||||||||||||||||||
| 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
+292
to
+297
|
||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||
| * 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
+301
to
+308
|
||||||||||||||||||||||||||||||||||
| * 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.