Ref.String now emits raw tabs instead of escaped "\t" after OPA V1.0.0

[xubinzheng]

Short description

After some investigation, it looks like after #7172, Ref.String() in ast/term.go (around line 1124) stopped using p.String() (which produces a JSON-escaped literal, it uses strconv.Quote(string(str)) under the covers) and instead writes the raw contents of the String term.

As a result, sequences like "\t" in a reference key now become an actual tab (ASCII 9) instead of the two‐byte escape (\ + t, ASCII 92,116).

Why this is a problem

The release notes don’t mention this change, so it appears to be an unintended regression. Existing code that depended on Ref.String() producing a properly escaped Go/JSON identifier will now receive raw control characters (e.g. ASCII 9 for \t), breaking downstream consumers that expect escape sequences.
Note: I think this also applies to \n or other escaped keys, which supports multiline rules, etc...

Details:

• OPA version: v1.0.0
• Example query, input, data, and policy that OPA was given: sample code in "Steps To Reproduce"
• Example output that OPA returned: sample code and output in "Steps To Reproduce"

Steps To Reproduce

// Before PR #7172 (OPA v0.68.0)
ref := ast.MustParseRef(`data.policy["ma\tin"]`)
fmt.Printf("%v\n", []byte(ref.String()))
// → [100 97 116 97 46 112 111 108 105 99 121 91 34 109 **97 92 116** 105 110 34 93]

// After PR #7172 (OPA v1.0.0)
ref := ast.MustParseRef(`data.policy["ma\tin"]`)
fmt.Printf("%v\n", []byte(ref.String()))
// → [100 97 116 97 46 112 111 108 105 99 121 91 34 109 **97 9** 105 110 34 93]

More details

This problem surfaced when I called:

ast.ParseBodyWithOpts(ref.String(), ast.ParserOptions{SkipRules: true})

The parser (see

opa/v1/ast/parser.go

Line 1651 in c99bf28

p.errorf(p.s.Loc(), "illegal string literal: %s", p.s.lit)

) then errors out with:

parsing rego query: 1 error occurred:
1:13: rego_parse_error: illegal string literal: "ma	in"
    data.policy["ma	in"]

Rather than changing Ref.String()—which has been released for some time, maybe we can update func (p *Parser) parseString() *Term so it accepts both escaped (\t) and raw control characters in string literals.

[anderseknert]

Thanks for that detailed report! Yeah, that does seem like an oversight. This function is called a lot, so avoiding strconv.Quote is desirable if possible as it allocates. I wonder if we can simply check for \ in the string and use it only when that's present?

[xubinzheng]

@anderseknert sounds good, as you suggested, I have put a PR together. Please let me know if you have any feedback.

#7552