Fix string fields w/ trailing slashes #7679
Merged
Conversation
This file contains 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
A string field w/ a trailing slash before the quote would parse incorrectly because the quote would be seen as escaped. We have to treat \\ as an escape sequence within strings in order to handle this.
e-dard
reviewed
Dec 2, 2016
| // Only escape char for a field value is a double-quote | ||
| if buf[i] == '\\' && i+1 < len(buf) && buf[i+1] == '"' { | ||
| // Only escape char for a field value is a double-quote and backslash | ||
| if buf[i] == '\\' && i+1 < len(buf) && (buf[i+1] == '"' || buf[i+1] == '\\') { |
There was a problem hiding this comment.
From recollection of when I did the er-escape-backslash branch, I don't think this quite captures the desirable behaviour.
If the rule is "the only escapable characters are a double quote and a backslash" then what happens if we hit a backslash that isn't followed by a double quote or a backslash. For example:
cpu value="foo\s"
This current approach I think will skip over the single slash silently, but I feel like maybe it should be an error? Possibly we have discussed this before..
There was a problem hiding this comment.
It think it should skip over the single \. If it returns an error, that's going to break existing clients.
e-dard
approved these changes
Dec 2, 2016
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.
Required for all non-trivial PRs
A string field w/ a trailing slash before the quote would parse incorrectly
because the quote would be seen as escaped. We have to treat \ as an
escape sequence within strings in order to handle this.
@e-dard