Skip to content

Strict-server: no-content Visit*Response skips nullable/optional header gating #2349

@mromaszewicz

Description

@mromaszewicz

Summary

The no-content branch of all three strict-server interface templates renders
response headers unconditionally, without the .IsNullable / .IsOptional
gating that PR #2301 added to the typed-body branch. This affects:

  • pkg/codegen/templates/strict/strict-interface.tmpl (stdhttp/chi/gorilla)
  • pkg/codegen/templates/strict/strict-fiber-interface.tmpl
  • pkg/codegen/templates/strict/strict-iris-interface.tmpl

This is not drift between templates — all three templates share the same
gap — so it was intentionally excluded from the scope of #2331.

Where

In each template, the {{if eq 0 (len .Contents)}} block emits a
Visit*Response that loops over $headers and calls the framework's
header-setter directly with fmt.Sprint(response.Headers.{{.GoName}}). For
example, stdhttp (strict-interface.tmpl lines 213–219):

func (response {{$opid}}{{$statusCode}}Response) Visit{{$opid}}Response(w http.ResponseWriter) error {
    {{range $headers -}}
        w.Header().Set("{{.Name}}", fmt.Sprint(response.Headers.{{.GoName}}))
    {{end -}}
    w.WriteHeader({{if $fixedStatusCode}}{{$statusCode}}{{else}}response.StatusCode{{end}})
    return nil
}

Fiber and iris have the equivalent block at the same point in their templates.

The typed-body branch immediately above this in strict-interface.tmpl (lines
95–107, 117–129, 140–152) uses the three-way switch:

{{if .IsNullable -}}
    if response.Headers.{{.GoName}}.IsSpecified() {
        w.Header().Set(\"{{.Name}}\", fmt.Sprint(response.Headers.{{.GoName}}.MustGet()))
    }
{{else if .IsOptional -}}
    if response.Headers.{{.GoName}} != nil {
        w.Header().Set(\"{{.Name}}\", fmt.Sprint(*response.Headers.{{.GoName}}))
    }
{{else -}}
    w.Header().Set(\"{{.Name}}\", fmt.Sprint(response.Headers.{{.GoName}}))
{{end -}}

The no-content branch does not.

Observable impact

Because the headers struct is defined once per response (top of
.Responses range, around line 32–38) and shared by both branches, its fields
already use {{.GoTypeDef}} in stdhttp — so for an optional header, the field
type is *string and a bare fmt.Sprint on a nil pointer emits <nil>. For a
nullable header (using runtime.Nullable[T]), fmt.Sprint of an unspecified
value emits whatever the type's default String() produces, not an omitted
header.

So a 204-style response declaring an optional or nullable header — and the
caller leaving it unset — will produce a wrong header value rather than no
header at all.

No existing test triggers this

In internal/test/strict-server/strict-schema.yaml, the only operation that
declares optional-header / nullable-header (HeadersExample) returns a
content-bearing JSON response, which goes through the typed-body branch and is
correctly gated. There is no fixture exercising the no-content + optional/
nullable header combination, which is why the bug hasn't been caught.

Suggested fix

Apply the same three-way .IsNullable / .IsOptional / default switch from
the typed-body branches to the no-content branch in all three templates. The
mechanical shape mirrors what PR #2301 did, just with the framework-specific
header-setter call (w.Header().Set / ctx.Response().Header.Set /
ctx.ResponseWriter().Header().Set).

A regression fixture should accompany the change — a no-content (e.g. 204)
operation declaring at least one optional and one nullable response header,
asserting that the header is omitted from the response when the value is unset.

Context

Surfaced while reviewing #2331 (fiber/iris drift from stdhttp). The drift fix
in #2331 brings fiber and iris to parity with stdhttp's current state,
which means they inherit this gap. Filing separately so the fix can be
reviewed on its own and includes the regression fixture this code path
currently lacks.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions