Fix template string not serialized with { escaped
#8161
Conversation
✅ Deploy Preview for openpolicyagent ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
v1/ast/term.go
Outdated
|
|
||
| errFindNotFound = errors.New("find: not found") | ||
|
|
||
| unescapedLeftCurly = regexp.MustCompile(`([^\\])({)`) |
There was a problem hiding this comment.
Why capture? Should this not be the same?
| unescapedLeftCurly = regexp.MustCompile(`([^\\])({)`) | |
| unescapedLeftCurly = regexp.MustCompile(`[^\\]{`) |
Hmm looking at this without the (), it does look kind of ambiguous. Is that why?
There was a problem hiding this comment.
That'll give Expected "unescaped \\{ left curly brace." but got "unescaped\\{ left curly brace."
i.e. space removed for reasons I didn't care enough about to find out about. I don't like the regex thing either, so I'll see if I can get something else to work.
There was a problem hiding this comment.
Turns out both solutions were flawed, as they would not escape a curly brace at position 0 in a string. Will cook something better up tonight.
srenatus
left a comment
srenatus
left a comment
There was a problem hiding this comment.
I kinda feel like we don't need a regex to find a { not preceded by a \. 🤔 But I suppose you have considered alternatives already.
|
@srenatus it was 3 AM, and went with the first thing I could think of. I'll see if there's something cheaper. |
Escaping `{` only at the time of serializtion, which should help
avoid special case treatment of these string values. But if there
is a better approach I haven't thought of, let me know.
Fixes open-policy-agent#8156
Signed-off-by: Anders Eknert <[email protected]>
anderseknert
force-pushed
the
templatestrings-esc-brace
branch
from
328e2fc to
b782fa5
Compare
|
@srenatus sorry about the delay.. turns out there is little time for coding when you are alone with two kids. I have just pushed an updated version that doesn't use a regular expression, and while it performs much better, it's more importantly correct. At least for any case I managed to come up with. PTAL, so that we can bump our Regal OPA dep and have a release first thing in the new year 🙂 |
…t#8161) Escaping `{` only at the time of serializtion, which should help avoid special case treatment of these string values. But if there is a better approach I haven't thought of, let me know. Fixes open-policy-agent#8156 Signed-off-by: Anders Eknert <[email protected]> (cherry picked from commit b06737b)
2 participants
Escaping
{only at the time of serializtion, which should help avoid special case treatment of these string values. But if there is a better approach I haven't thought of, let me know.Fixes #8156