Skip to content

Commit

Permalink
all: make HTML escaping in JSON an opt-in
Browse files Browse the repository at this point in the history
We should avoid calling encoding/json.Marshal where possible.
It does not allow passing in options, and it escapes HTML.
This means that it forces us to always escape HTML by default,
even in cases where we want to not escape by default.

One such case are our MarshalJSON methods,
which always performed HTML escaping without a choice.
This caused the command `cue export` to always escape,
even when its --escape flag wasn't being set.

As can be seen in the updated test, `cue export` no longer escapes.
The --escape flag is now plumbed through as well,
which makes the top-level encoding/json.Encoder.Encode call perform the
necessary escaping when the flag is set.

Another case is cuelang.org/go/pkg/encoding/json,
which has an HTMLEscape API rather than a boolean option,
so Marshal and MarshalStream should not perform any escaping.

This change may break users of `cue export` or `json.Marshal` who did
depend on the HTML escaping to happen by default.
However, we have always had documented flags and APIs to enable the HTML
escaping, and before this fix, there wasn't any way to not escape HTML.
For those reasons, this should be an acceptable change to make.

To get encoding/json.Marshal's behavior without the escaping,
create a new Marshal func in internal/encoding/json
which uses encoding/json.Encoder.Encode with a buffer.

Note that we import internal/encoding/json as "internaljson",
to make its use instead of encoding/json clear and explicit.
For consistency, all calls to encoding/json.Marshal are replaced.

If encoding/json's API is ever improved upon,
we can likely simplify or avoid our workaround.
For now, avoiding upstream's Marshal API is the best we can do.

Fixes #1243.

Signed-off-by: Daniel Martí <mvdan@mvdan.cc>
Change-Id: Ie11c8cfdc3741927c2aae28bce0d67c214411480
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/548783
Reviewed-by: Roger Peppe <rogpeppe@gmail.com>
Unity-Result: CUEcueckoo <cueckoo@cuelang.org>
TryBot-Result: CUEcueckoo <cueckoo@cuelang.org>
  • Loading branch information
mvdan committed Apr 7, 2023
1 parent 1aefd09 commit 48207fb
Show file tree
Hide file tree
Showing 10 changed files with 42 additions and 24 deletions.
1 change: 1 addition & 0 deletions cmd/cue/cmd/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -720,6 +720,7 @@ func (b *buildPlan) parseFlags() (err error) {
PkgName: flagPackage.String(b.cmd),
Strict: flagStrict.Bool(b.cmd),
InlineImports: flagInlineImports.Bool(b.cmd),
EscapeHTML: flagEscape.Bool(b.cmd),
}
return nil
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/cue/cmd/testdata/script/export_escape.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ specialHTML: "& < >"
{
"simple": "hello",
"specialJSON": "\\ \"",
"specialHTML": "\u0026 \u003c \u003e"
"specialHTML": "& < >"
}
-- stdout-escape.golden --
{
Expand Down
4 changes: 2 additions & 2 deletions cue/testdata/gen.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ package main

import (
"bytes"
"encoding/json"
"flag"
"fmt"
"go/ast"
Expand All @@ -36,6 +35,7 @@ import (
cueformat "cuelang.org/go/cue/format"
"cuelang.org/go/cue/parser"
"cuelang.org/go/internal"
internaljson "cuelang.org/go/internal/encoding/json"
"cuelang.org/go/pkg/encoding/yaml"
"cuelang.org/go/tools/fix"
)
Expand Down Expand Up @@ -307,7 +307,7 @@ func (e *extractor) populate(src []byte) {
e.a.Files = append(e.a.Files,
txtar.File{Name: "out/yaml", Data: []byte(s)})

b, err := json.Marshal(v)
b, err := internaljson.Marshal(v)
if err != nil {
fmt.Fprintln(e.header, "#bug: true")
e.warnf("Could not encode as JSON: %v", err)
Expand Down
17 changes: 9 additions & 8 deletions cue/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import (
"cuelang.org/go/internal/core/runtime"
"cuelang.org/go/internal/core/subsume"
"cuelang.org/go/internal/core/validate"
internaljson "cuelang.org/go/internal/encoding/json"
"cuelang.org/go/internal/types"
)

Expand Down Expand Up @@ -147,13 +148,13 @@ func (o *structValue) marshalJSON() (b []byte, err errors.Error) {
n := o.Len()
for i := 0; i < n; i++ {
k, v := o.At(i)
s, err := json.Marshal(k)
s, err := internaljson.Marshal(k)
if err != nil {
return nil, unwrapJSONError(err)
}
b = append(b, s...)
b = append(b, ':')
bb, err := json.Marshal(v)
bb, err := internaljson.Marshal(v)
if err != nil {
return nil, unwrapJSONError(err)
}
Expand Down Expand Up @@ -292,7 +293,7 @@ func marshalList(l *Iterator) (b []byte, err errors.Error) {
b = append(b, '[')
if l.Next() {
for i := 0; ; i++ {
x, err := json.Marshal(l.Value())
x, err := internaljson.Marshal(l.Value())
if err != nil {
return nil, unwrapJSONError(err)
}
Expand Down Expand Up @@ -912,7 +913,7 @@ func (v Value) MarshalJSON() (b []byte, err error) {
func (v Value) marshalJSON() (b []byte, err error) {
v, _ = v.Default()
if v.v == nil {
return json.Marshal(nil)
return internaljson.Marshal(nil)
}
ctx := newContext(v.idx)
x := v.eval(ctx)
Expand All @@ -927,17 +928,17 @@ func (v Value) marshalJSON() (b []byte, err error) {
// TODO: implement marshalles in value.
switch k := x.Kind(); k {
case adt.NullKind:
return json.Marshal(nil)
return internaljson.Marshal(nil)
case adt.BoolKind:
return json.Marshal(x.(*adt.Bool).B)
return internaljson.Marshal(x.(*adt.Bool).B)
case adt.IntKind, adt.FloatKind, adt.NumKind:
b, err := x.(*adt.Num).X.MarshalText()
b = bytes.TrimLeft(b, "+")
return b, err
case adt.StringKind:
return json.Marshal(x.(*adt.String).Str)
return internaljson.Marshal(x.(*adt.String).Str)
case adt.BytesKind:
return json.Marshal(x.(*adt.Bytes).B)
return internaljson.Marshal(x.(*adt.Bytes).B)
case adt.ListKind:
i, _ := v.List()
return marshalList(&i)
Expand Down
6 changes: 3 additions & 3 deletions encoding/openapi/openapi.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
package openapi

import (
"encoding/json"
"fmt"
"strings"

Expand All @@ -24,6 +23,7 @@ import (
"cuelang.org/go/cue/errors"
"cuelang.org/go/cue/token"
cuejson "cuelang.org/go/encoding/json"
internaljson "cuelang.org/go/internal/encoding/json"
)

// A Config defines options for converting CUE to and from OpenAPI.
Expand Down Expand Up @@ -95,7 +95,7 @@ func Gen(inst cue.InstanceOrValue, c *Config) ([]byte, error) {
if err != nil {
return nil, err
}
return json.Marshal(all)
return internaljson.Marshal(all)
}

// Generate generates the set of OpenAPI schema for all top-level types of the
Expand Down Expand Up @@ -128,7 +128,7 @@ func (g *Generator) All(inst cue.InstanceOrValue) (*OrderedMap, error) {
}

func toCUE(name string, x interface{}) (v ast.Expr, err error) {
b, err := json.Marshal(x)
b, err := internaljson.Marshal(x)
if err == nil {
v, err = cuejson.Extract(name, b)
}
Expand Down
4 changes: 2 additions & 2 deletions encoding/openapi/orderedmap.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import (
"cuelang.org/go/cue/ast"
"cuelang.org/go/cue/literal"
"cuelang.org/go/cue/token"
"cuelang.org/go/internal/encoding/json"
internaljson "cuelang.org/go/internal/encoding/json"
)

// An OrderedMap is a set of key-value pairs that preserves the order in which
Expand Down Expand Up @@ -177,5 +177,5 @@ func (m *OrderedMap) getMap(key string) *OrderedMap {
func (m *OrderedMap) MarshalJSON() (b []byte, err error) {
// This is a pointer receiever to enforce that we only store pointers to
// OrderedMap in the output.
return json.Encode((*ast.StructLit)(m))
return internaljson.Encode((*ast.StructLit)(m))
}
3 changes: 2 additions & 1 deletion internal/core/convert/go.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import (
"cuelang.org/go/internal"
"cuelang.org/go/internal/core/adt"
"cuelang.org/go/internal/core/compile"
internaljson "cuelang.org/go/internal/encoding/json"
"cuelang.org/go/internal/types"
)

Expand Down Expand Up @@ -304,7 +305,7 @@ func convertRec(ctx *adt.OpContext, nilIsTop bool, x interface{}) adt.Value {
if err != nil {
return ctx.AddErr(errors.Promote(err, "encoding.TextMarshaler"))
}
b, err = json.Marshal(string(b))
b, err = internaljson.Marshal(string(b))
if err != nil {
return ctx.AddErr(errors.Promote(err, "json"))
}
Expand Down
20 changes: 17 additions & 3 deletions internal/encoding/json/encode.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,21 @@ import (
"cuelang.org/go/internal/astinternal"
)

// Encode converts a CUE AST to JSON.
// Marshal is a replacement for [json.Marshal] without HTML escaping.
func Marshal(v any) ([]byte, error) {
var buf bytes.Buffer
enc := json.NewEncoder(&buf)
enc.SetEscapeHTML(false)
if err := enc.Encode(v); err != nil {
return nil, err
}
p := buf.Bytes()
// Unlike json.Marshal, json.Encoder.Encode adds a trailing newline.
p = bytes.TrimSuffix(p, []byte("\n"))
return p, nil
}

// Encode converts a CUE AST to unescaped JSON.
//
// The given file must only contain values that can be directly supported by
// JSON:
Expand Down Expand Up @@ -187,7 +201,7 @@ func (e *encoder) encodeScalar(l *ast.BasicLit, allowMinus bool) error {
if err != nil {
return err
}
b, err := json.Marshal(str)
b, err := Marshal(str)
if err != nil {
return err
}
Expand Down Expand Up @@ -277,7 +291,7 @@ func (e *encoder) encodeDecls(decls []ast.Decl, endPos token.Pos) error {
if err != nil {
return errors.Newf(x.Label.Pos(), "json: only literal labels allowed")
}
b, err := json.Marshal(name)
b, err := Marshal(name)
if err != nil {
return err
}
Expand Down
5 changes: 3 additions & 2 deletions pkg/encoding/json/manual.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"cuelang.org/go/cue/parser"
"cuelang.org/go/cue/token"
cuejson "cuelang.org/go/encoding/json"
internaljson "cuelang.org/go/internal/encoding/json"
)

// Compact generates the JSON-encoded src with insignificant space characters
Expand Down Expand Up @@ -70,7 +71,7 @@ func HTMLEscape(src []byte) string {

// Marshal returns the JSON encoding of v.
func Marshal(v cue.Value) (string, error) {
b, err := json.Marshal(v)
b, err := internaljson.Marshal(v)
return string(b), err
}

Expand All @@ -83,7 +84,7 @@ func MarshalStream(v cue.Value) (string, error) {
}
buf := &bytes.Buffer{}
for iter.Next() {
b, err := json.Marshal(iter.Value())
b, err := internaljson.Marshal(iter.Value())
if err != nil {
return "", err
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/encoding/json/testdata/gen.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -65,10 +65,10 @@ t9: """
{"b":2}

"""
t10: "{\"a\":\"\\\\ \\\" \\u0026 \\u003c \\u003e\"}"
t10: "{\"a\":\"\\\\ \\\" & < >\"}"
t11: "{\"a\":\"\\\\ \\\" \\u0026 \\u003c \\u003e\"}"
t12: """
{"a":"\\\\ \\" \\u0026 \\u003c \\u003e"}
{"a":"\\\\ \\" & < >"}
{"b":""}

"""
Expand Down

0 comments on commit 48207fb

Please sign in to comment.