Skip to content

fix: skip URL rewriting for non-relation field values - #183

Merged
dunglas merged 1 commit into
dunglas:mainfrom
Nayte91:fix/fields-query-value-encoding
Jul 25, 2026
Merged

fix: skip URL rewriting for non-relation field values#183
dunglas merged 1 commit into
dunglas:mainfrom
Nayte91:fix/fields-query-value-encoding

Conversation

@Nayte91

@Nayte91 Nayte91 commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

Problem

Fixes #96.

A ?fields= query that selects scalar fields returns corrupted values:

  • plain strings are percent-encoded — "Book 1""Book%201", "Roland Garros 2019""Roland%20Garros%202019";
  • numbers are coerced to strings — 2019"2019" (a float is even truncated).

The bug only affects the query form (?fields=), not the header form (Fields:), which is why it surfaced with parametrized queries.

Root cause

In Apply, the relation handler is invoked for every scalar leaf of the document — including pure field projections that are not relations. Each such value is parsed with url.Parse and re-serialized with url.URL.String(), which percent-encodes the path; numbers are additionally re-marshalled from a string. Only the query form rewrites the body (urlRewriter + u.String()), so only it exposes the corruption.

Fix

Guard the handler so a node that is not a relation — no relation to push (preload) and no child selector to propagate — is returned untouched, before any URL machinery runs:

if !n.preload && !n.hasChildren(preload) && !n.hasChildren(fields) {
    return ""
}

The query-vs-header propagation logic is left intact; the guard only restores the precondition that block always assumed — that it operates on relations.

Efficiency

Because the guard short-circuits before url.Parse, the query re-encoding and the json.Marshal, it also removes wasted work on the fields path. Benchmark on a ?fields= query selecting 7 scalar fields (5 runs, Go 1.26):

Metric Before After Δ
Time ~17,600 ns/op ~12,785 ns/op −27%
Memory 14,220 B/op 11,512 B/op −19%
Allocations 175 allocs/op 122 allocs/op −30%
Benchmark used
func BenchmarkApplyFieldsQuery(b *testing.B) {
	v := New()
	body := []byte(`{"a":"Roland Garros 2019","b":"Internationaux de France de tennis","c":"A good long description here","d":"Some more textual content","e":"Yet another value with spaces","f":2019,"g":42,"h":"short","i":"value","j":"tail"}`)
	headers := http.Header{"Content-Type": []string{"application/json"}}

	b.ReportAllocs()
	b.ResetTimer()
	for i := 0; i < b.N; i++ {
		req := httptest.NewRequest("GET", "/", nil)
		req.URL.RawQuery = `fields="/a","/b","/c","/d","/e","/f","/g"`
		rw := httptest.NewRecorder()
		if _, err := v.Apply(req, rw, bytes.NewReader(body), headers.Clone()); err != nil {
			b.Fatal(err)
		}
	}
}

Tests

Two regression tests added in server_test.go (a numeric year field was added to the JSON-LD fixture to cover type preservation):

  • TestFieldsQueryDoesNotEncodePlainValues"Book 1" stays "Book 1";
  • TestFieldsQueryPreservesNumberType2019 stays a JSON number.

A "?fields=" query selecting scalar fields returned corrupted values:
plain strings were percent-encoded ("Book 1" -> "Book%201") and numbers
were coerced to strings (2019 -> "2019"). The header form was unaffected
because it never rewrites the response body.

The relation handler ran for every scalar leaf, including pure field
projections that are not relations. Those values were parsed with
url.Parse and re-serialized via url.URL.String(), which percent-encodes
the path and stringifies numbers.

Guard the handler so a node with no relation to push and no child
selector to propagate is left untouched. Besides fixing the corruption,
this skips url.Parse, the query re-encoding and the json.Marshal for
every projected scalar, cutting allocations on the fields path by ~30%.

Fixes #96
@CLAassistant

CLAassistant commented Jul 25, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@dunglas
dunglas merged commit c102ddb into dunglas:main Jul 25, 2026
6 checks passed
@dunglas

dunglas commented Jul 25, 2026

Copy link
Copy Markdown
Owner

Good catch! Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Response's body is percent encoded when parametrized queries

3 participants