-
-
Notifications
You must be signed in to change notification settings - Fork 991
fix(core): escape dotted keys in flattenAttributes to correct log rendering #2987
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
Changes from all commits
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 |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "@trigger.dev/core": patch | ||
| --- | ||
|
|
||
| Fix issue where logging objects with keys containing dots resulted in incorrect nested object structure in logs (#1510) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,6 +5,36 @@ export const CIRCULAR_REFERENCE_SENTINEL = "$@circular(("; | |
|
|
||
| const DEFAULT_MAX_DEPTH = 128; | ||
|
|
||
| function escapeKey(key: string) { | ||
| return key.replace(/\\/g, "\\\\").replace(/\./g, "\\."); | ||
| } | ||
|
|
||
| function splitPath(path: string): string[] { | ||
| const parts: string[] = []; | ||
| let current = ""; | ||
| let isEscaped = false; | ||
|
|
||
| for (let i = 0; i < path.length; i++) { | ||
| const char = path[i]; | ||
|
|
||
| if (isEscaped) { | ||
| current += char; | ||
| isEscaped = false; | ||
| } else if (char === "\\") { | ||
| isEscaped = true; | ||
| } else if (char === ".") { | ||
| parts.push(current); | ||
| current = ""; | ||
| } else { | ||
| current += char; | ||
| } | ||
| } | ||
|
|
||
| parts.push(current); | ||
|
|
||
| return parts; | ||
| } | ||
|
Comment on lines
+12
to
+36
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Minor edge case: trailing backslash is silently dropped. If a path ends with an unescaped backslash (e.g., This is unlikely to occur in practice since 🛡️ Optional defensive fix parts.push(current);
+ if (isEscaped) {
+ // Handle trailing backslash (shouldn't occur with proper escaping)
+ parts[parts.length - 1] += "\\";
+ }
+
return parts;
}🤖 Prompt for AI Agents |
||
|
|
||
| export function flattenAttributes( | ||
| obj: unknown, | ||
| prefix?: string, | ||
|
|
@@ -24,7 +54,7 @@ class AttributeFlattener { | |
| constructor( | ||
| private maxAttributeCount?: number, | ||
| private maxDepth: number = DEFAULT_MAX_DEPTH | ||
| ) {} | ||
| ) { } | ||
|
|
||
| get attributes(): Attributes { | ||
| return this.result; | ||
|
|
@@ -200,7 +230,8 @@ class AttributeFlattener { | |
| break; | ||
| } | ||
|
|
||
| const newPrefix = `${prefix ? `${prefix}.` : ""}${Array.isArray(obj) ? `[${key}]` : key}`; | ||
| const newPrefix = `${prefix ? `${prefix}.` : ""}${Array.isArray(obj) ? `[${key}]` : escapeKey(key) | ||
| }`; | ||
|
|
||
| if (Array.isArray(value)) { | ||
| for (let i = 0; i < value.length; i++) { | ||
|
|
@@ -278,7 +309,7 @@ export function unflattenAttributes( | |
| continue; | ||
| } | ||
|
|
||
| const parts = key.split(".").reduce( | ||
| const parts = splitPath(key).reduce( | ||
| (acc, part) => { | ||
| if (part.startsWith("[") && part.endsWith("]")) { | ||
| // Handle array indices more precisely | ||
|
|
||
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.
🔴 Map keys containing dots are not escaped, causing incorrect unflattening
The PR fixes dotted keys for regular objects by using
escapeKey()at line 233, but Map keys at line 150 are not escaped.Click to expand
How the bug is triggered
When a Map has a key containing a dot (e.g.,
"Key 0.002mm"), the current code builds the path without escaping:Compare to regular object keys at line 233:
Actual vs Expected behavior
Actual: Map with key
"Key 0.002mm"produces flattened pathmyMap.Key 0.002mm, whichsplitPath()splits into['myMap', 'Key 0', '002mm'], resulting in nested object{ myMap: { 'Key 0': { '002mm': value } } }Expected: Path should be
myMap.Key 0\.002mm, whichsplitPath()correctly splits into['myMap', 'Key 0.002mm'], preserving the original structure{ myMap: { 'Key 0.002mm': value } }Impact
This causes data corruption during the flatten/unflatten cycle for Maps with dotted keys - the exact same issue the PR was trying to fix, but only fixed for regular objects.
(Refers to line 150)
Recommendation: Escape the Map key using
escapeKey():this.#processValue(value, \${prefix || "map"}.${escapeKey(keyStr)}`, depth);`Was this helpful? React with 👍 or 👎 to provide feedback.