zalando / skipper / 39278377840

81%

Build Type

push

github

Committed by web-flow

Commit Message

fix: headerToQuery clears the query param when the header is missing (#4348)

## Symptom

`headerToQuery` changes the query even when the request doesn't have the
header:

```
r: * -> headerToQuery("X-Foo-Header", "foo-query-param") -> "http://backend"
```

| Request (no `X-Foo-Header`) | query sent to the backend on master |
expected |
| --- | --- | --- |
| `/` | `?foo-query-param=` | no query |
| `/?foo-query-param=baz` | `?foo-query-param=` | `?foo-query-param=baz`
|
| `/?b=2&a=1` | `?a=1&b=2&foo-query-param=` | `?b=2&a=1` |

The second row is the worst one: a value the client sent in the query is
replaced with an empty string just because the header is absent.

## Cause

`Request` always calls `params.Set(f.queryParamName,
req.Header.Get(f.headerName))` and re-encodes the query. `Header.Get`
returns `""` for a missing header, so the param is set to an empty
value.

`queryToHeader`, the reverse filter, already returns early when the
query param is missing.

## Change

- Return without touching the query when the request has no values for
the header (`len(req.Header.Values(name)) == 0`). A header that is
present, even with an empty value, is still copied as before.
- One sentence in `docs/reference/filters.md` saying the query is left
unchanged without the header.

## Tests

New `TestHeaderToQueryFilter_RequestWithoutHeader` covers the three rows
above. On master it fails:

```
headertoquery_test.go:50: request without the header: got https://example.org?foo-query-param=, want https://example.org unchanged
headertoquery_test.go:50: request without the header: got https://example.org?foo-query-param=, want https://example.org?foo-query-param=baz unchanged
headertoquery_test.go:50: request without the header: got https://example.org?a=1&b=2&foo-query-param=, want https://example.org?b=2&a=1 unchanged
```

With the change, `go test ./filters/builtin/` passes, and `gofmt` and
`go vet` are clean.

This change was prepared with AI assistance and reviewed/... (continued)

Coverage Stats

4 of 4 new or added lines in 1 file covered. (100.0%)

10 existing lines in 5 files now uncovered.

27713 of 34198 relevant lines covered (81.04%)

73803.49 hits per line