Skip to content
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

parse_logfmt adds extra fields #725

Closed
ofirmad opened this issue Jan 8, 2024 · 3 comments · Fixed by #736
Closed

parse_logfmt adds extra fields #725

ofirmad opened this issue Jan 8, 2024 · 3 comments · Fixed by #736
Labels
type: bug A code related bug vrl: stdlib Changes to the standard library

Comments

@ofirmad
Copy link

ofirmad commented Jan 8, 2024

A note for the community

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment

Problem

I use parse_logfmt and I get field which I didn't expect to get.

in the following link you can see "\t\t"": true in the output. This field should be part of errorVerbose value, it should be standalone field.
https://playground.vrl.dev/?state=eyJwcm9ncmFtIjoibG9nLCBlcnIgID0gIHBhcnNlX2xvZ2ZtdCgubWVzc2FnZSlcbi4gPSBtZXJnZSguLCBsb2cpIiwiZXZlbnQiOnsibWVzc2FnZSI6ImxldmVsPWVycm9yIGVycm9yVmVyYm9zZT1cImNhbm5vdCBcXHRcXHRcIiJ9LCJpc19qc29ubCI6ZmFsc2UsImVycm9yIjpudWxsfQ%3D%3D

but if I take one '' off I get a good output, without extra fields
https://playground.vrl.dev/?state=eyJwcm9ncmFtIjoibG9nLCBlcnIgID0gIHBhcnNlX2xvZ2ZtdCgubWVzc2FnZSlcbi4gPSBtZXJnZSguLCBsb2cpIiwiZXZlbnQiOnsibWVzc2FnZSI6ImxldmVsPWVycm9yIGVycm9yVmVyYm9zZT1cImNhbm5vdCBcdFxcdFwiIn0sImlzX2pzb25sIjpmYWxzZSwiZXJyb3IiOm51bGx9

I would expect to get errorVerbose value as one line value, without being split, for example:
{
"errorVerbose": "cannot \t\t",
"level": "error",
"message": "level=error errorVerbose="cannot \t\t""
}

I can see the extra fields is added when there is \x\y pattern inside the value in the input (x is a single character, y can be any string).

Thanks.

Configuration

No response

Version

0.24.1

Debug Output

No response

Example Data

No response

Additional Context

No response

References

No response

@ofirmad ofirmad added the type: bug A code related bug label Jan 8, 2024
@drmason13
Copy link
Contributor

drmason13 commented Mar 4, 2024

I had a look into this, and agree it doesn't seem like the correct behavior.

I'd be interested in trying to fix it, so far I've found that vrl itself defines the parse_logfmt function in its own stdlib in the vrl repo

EDIT: and the meat of it is actually in a key value parser written in nom

@jszwedko jszwedko transferred this issue from vectordotdev/vector Mar 4, 2024
@jszwedko jszwedko added the vrl: stdlib Changes to the standard library label Mar 4, 2024
@drmason13
Copy link
Contributor

I found perhaps the most minimal example VRL playground:

"a=\"1 \\\""

will output

{
	"\\\"": true,
	"a": "\"1",
	"message": "a=\"1 \\\""
}

@drmason13
Copy link
Contributor

I've got a reproduction in a simple local project with just the relevant nom parsers copied across and vector dependencies removed.
Should be able to figure this out with enough time.

As far as I can tell it's caused by the standalone key option, when that's true something goes a bit awry in the parsing.

drmason13 added a commit to drmason13/vrl that referenced this issue Mar 10, 2024
drmason13 added a commit to drmason13/vrl that referenced this issue Mar 10, 2024
drmason13 added a commit to drmason13/vrl that referenced this issue Mar 11, 2024
github-merge-queue bot pushed a commit that referenced this issue Mar 13, 2024
* fix(parse_key_value): handle more escapes (#725)

no longer accepts empty keys!

* fix(parse_key_value): always refuse to parse empty keys

this behavior ought to be consistent whether or not standalone_keys is true

* changelog fragment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug A code related bug vrl: stdlib Changes to the standard library
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants