Skip to content

Commit

Permalink
[pkg/ottl] Fix escape sequences in strings (open-telemetry#30839)
Browse files Browse the repository at this point in the history
**Description:** <Describe what has changed.>
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
Fix ottl parsing to properly handle escape sequences.
@TylerHelmuth

**Link to tracking Issue:**
open-telemetry#23238

**Testing:** <Describe what testing was performed and which tests were
added.>
Added a new unit test with the example from the linked issue

**Documentation:** <Describe the documentation added.>
None (pure bugfix)
  • Loading branch information
quentinmit committed Jan 31, 2024
1 parent 3f48044 commit ed487b3
Show file tree
Hide file tree
Showing 5 changed files with 75 additions and 21 deletions.
27 changes: 27 additions & 0 deletions .chloggen/ottl-string.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# Use this changelog template to create an entry for release notes.

# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: bug_fix

# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
component: pkg/ottl

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Fix parsing of string escapes in OTTL

# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
issues: [23238]

# (Optional) One or more lines of additional information to render under the primary note.
# These lines will be padded with 2 spaces and then inserted directly into the document.
# Use pipe (|) for multiline entries.
subtext:

# If your change doesn't affect end users or the exported elements of any package,
# you should instead start your pull request title with [chore] or use the "Skip Changelog" label.
# Optional: The change log or logs in which this entry should be included.
# e.g. '[user]' or '[user, api]'
# Include 'user' if the change is relevant to end users.
# Include 'api' if there is a change to a library API.
# Default: '[user]'
change_logs: []
38 changes: 38 additions & 0 deletions pkg/ottl/e2e/e2e_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -518,6 +518,44 @@ func Test_e2e_converters(t *testing.T) {
tCtx.GetLogRecord().Attributes().PutStr("test", "pass")
},
},
{
statement: `set(attributes["test"], "\\")`,
want: func(tCtx ottllog.TransformContext) {
tCtx.GetLogRecord().Attributes().PutStr("test", "\\")
},
},
{
statement: `set(attributes["test"], "\\\\")`,
want: func(tCtx ottllog.TransformContext) {
tCtx.GetLogRecord().Attributes().PutStr("test", "\\\\")
},
},
{
statement: `set(attributes["test"], "\\\\\\")`,
want: func(tCtx ottllog.TransformContext) {
tCtx.GetLogRecord().Attributes().PutStr("test", "\\\\\\")
},
},
{
statement: `set(attributes["test"], "\\\\\\\\")`,
want: func(tCtx ottllog.TransformContext) {
tCtx.GetLogRecord().Attributes().PutStr("test", "\\\\\\\\")
},
},
{
statement: `set(attributes["test"], "\"")`,
want: func(tCtx ottllog.TransformContext) {
tCtx.GetLogRecord().Attributes().PutStr("test", `"`)
},
},
{
statement: `keep_keys(attributes["foo"], ["\\", "bar"])`,
want: func(tCtx ottllog.TransformContext) {
// keep_keys should see two arguments
m := tCtx.GetLogRecord().Attributes().PutEmptyMap("foo")
m.PutStr("bar", "pass")
},
},
}

for _, tt := range tests {
Expand Down
2 changes: 1 addition & 1 deletion pkg/ottl/grammar.go
Original file line number Diff line number Diff line change
Expand Up @@ -433,7 +433,7 @@ func buildLexer() *lexer.StatefulDefinition {
{Name: `Bytes`, Pattern: `0x[a-fA-F0-9]+`},
{Name: `Float`, Pattern: `[-+]?\d*\.\d+([eE][-+]?\d+)?`},
{Name: `Int`, Pattern: `[-+]?\d+`},
{Name: `String`, Pattern: `"(\\"|[^"])*"`},
{Name: `String`, Pattern: `"(\\.|[^\\"])*"`},
{Name: `OpNot`, Pattern: `\b(not)\b`},
{Name: `OpOr`, Pattern: `\b(or)\b`},
{Name: `OpAnd`, Pattern: `\b(and)\b`},
Expand Down
9 changes: 9 additions & 0 deletions pkg/ottl/lexer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,15 @@ func Test_lexer(t *testing.T) {
{"Bytes", "0x0102030405060708"},
{"RParen", ")"},
}},
{"string escape with trailing backslash", `a("\\", "b")`, false, []result{
{"Lowercase", "a"},
{"LParen", "("},
{"String", `"\\"`},
{"Punct", ","},
{"String", `"b"`},
{"RParen", ")"},
}},
{"string escape with mismatched backslash", `"\"`, true, nil},
{"Mixing case numbers and underscores", `aBCd_123E_4`, false, []result{
{"Lowercase", "a"},
{"Uppercase", "BC"},
Expand Down
20 changes: 0 additions & 20 deletions pkg/ottl/ottlfuncs/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -254,9 +254,6 @@ The `replace_all_matches` function replaces any matching string value with the r

Each string value in `target` that matches `pattern` will get replaced with `replacement`. Non-string values are ignored.

There is currently a bug with OTTL that does not allow the pattern to end with `\\"`.
[See Issue 23238 for details](https://github.com/open-telemetry/opentelemetry-collector-contrib/issues/23238).

Examples:

- `replace_all_matches(attributes, "/user/*/list/*", "/user/{userId}/list/{listId}")`
Expand All @@ -278,10 +275,6 @@ The `replacement` string can refer to matched groups using [regexp.Expand syntax

The `function` is an optional argument that can take in any Converter that accepts a (`replacement`) string and returns a string. An example is a hash function that replaces any matching regex pattern with the hash value of `replacement`.

There is currently a bug with OTTL that does not allow the pattern to end with `\\"`.
If your pattern needs to end with backslashes, add something inconsequential to the end of the pattern such as `{1}`, `$`, or `.*`.
[See Issue 23238 for details](https://github.com/open-telemetry/opentelemetry-collector-contrib/issues/23238).

Examples:

- `replace_all_patterns(attributes, "value", "/account/\\d{4}", "/account/{accountId}")`
Expand All @@ -305,9 +298,6 @@ If `target` matches `pattern` it will get replaced with `replacement`.

The `function` is an optional argument that can take in any Converter that accepts a (`replacement`) string and returns a string. An example is a hash function that replaces any matching glob pattern with the hash value of `replacement`.

There is currently a bug with OTTL that does not allow the pattern to end with `\\"`.
[See Issue 23238 for details](https://github.com/open-telemetry/opentelemetry-collector-contrib/issues/23238).

Examples:

- `replace_match(attributes["http.target"], "/user/*/list/*", "/user/{userId}/list/{listId}")`
Expand All @@ -327,10 +317,6 @@ The `replacement` string can refer to matched groups using [regexp.Expand syntax

The `function` is an optional argument that can take in any Converter that accepts a (`replacement`) string and returns a string. An example is a hash function that replaces a matching regex pattern with the hash value of `replacement`.

There is currently a bug with OTTL that does not allow the pattern to end with `\\"`.
If your pattern needs to end with backslashes, add something inconsequential to the end of the pattern such as `{1}`, `$`, or `.*`.
[See Issue 23238 for details](https://github.com/open-telemetry/opentelemetry-collector-contrib/issues/23238).

Examples:

- `replace_pattern(resource.attributes["process.command_line"], "password\\=[^\\s]*(\\s?)", "password=***")`
Expand Down Expand Up @@ -674,9 +660,6 @@ If target is not a string, it will be converted to one:

If target is nil, false is always returned.

There is currently a bug with OTTL that does not allow the target string to end with `\\"`.
[See Issue 23238 for details](https://github.com/open-telemetry/opentelemetry-collector-contrib/issues/23238).

Examples:

- `IsMatch(attributes["http.path"], "foo")`
Expand Down Expand Up @@ -918,9 +901,6 @@ The `Split` Converter separates a string by the delimiter, and returns an array

If the `target` is not a string or does not exist, the `Split` Converter will return an error.

There is currently a bug with OTTL that does not allow the target string to end with `\\"`.
[See Issue 23238 for details](https://github.com/open-telemetry/opentelemetry-collector-contrib/issues/23238).

Examples:

- `Split("A|B|C", "|")`
Expand Down

0 comments on commit ed487b3

Please sign in to comment.