fix(models): reject measurements and keys with leading underscores #20314
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This will likely break a lot of people's Kap workflows, because they tend to write fields like _start and _stop, iirc. |
docmerlin
approved these changes
Dec 14, 2020
| err = walkFields(fields, func(k, v []byte) bool { | ||
| if k[0] == '_' { |
There was a problem hiding this comment.
Do you need to check len(k)>0 here first?
There was a problem hiding this comment.
This one seems to be covered by a preceding check, I get "missing field key" if I try to parse a line with an empty key
| @@ -534,10 +551,13 @@ func scanKey(buf []byte, i int) (int, []byte, error) { | |||
| } | |||
|
|
|||
| // Iterate over tags keys ensure that we do not encounter any | |||
| // of the reserved tag keys such as _measurement or _field. | |||
| // of the reserved tag keys. | |||
| for j := 0; j < commas; j++ { | |||
| _, key := scanTo(buf[indices[j]:indices[j+1]-1], 0, '=') | |||
|
|
|||
There was a problem hiding this comment.
Do you need to check len(k)>0 here first?
before line 558 or is that impossible?
There was a problem hiding this comment.
This one seems to be covered by a preceding check, I get "missing tag key" if I try to parse cpu,=testing,tag=value value=2
|
Closing for now, needs more discussion |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #20310
This brings our implementation in line with the spec, and prevents panics when users override keys like
_start. The first commit is a refactoring that I could split into a separate PR, if desired. The diff in the test file shows the impact of the actual fix.