Skip to content

httpgen: compose JSON-mapping features into a single four-stage marshaler per message #254

Description

@SebastienMelki

Is your feature request related to a problem? Please describe.

httpgen has nine features that customize JSON output (int64_encoding, enum_value, bytes_encoding, timestamp_format, nullable, empty_behavior, flatten, oneof_config, unwrap). Each emits its own complete MarshalJSONSebuf for the message it applies to, so at most one may be used per message.

That restriction has no semantic basis. Eight of the nine generate the identical pipeline and mutate only the key of their own field:

// nullable                                  // timestamp_format
data, _ := opts.Marshal(x)                   data, _ := opts.Marshal(x)
json.Unmarshal(data, &raw)                   json.Unmarshal(data, &raw)

if x.F1 == nil {                             if x.F2 != nil {
    raw["f1"] = []byte("null")                   raw["f2"], _ = json.Marshal(t.Unix())
}                                            }

return json.Marshal(raw)                     return json.Marshal(raw)

Disjoint keys, same shape. Combining them is concatenating the middle blocks. The exclusivity is an artifact of code layout — one method per feature instead of one method per message.

Notably, the codebase already composes these annotations across nesting levels. From the generated flatten marshaler:

// Forward opts to child's MarshalJSONSebuf when available (annotation composability)

Composability is already an accepted design goal. It just isn't implemented within a message.

Why this matters

1. OpenAPI publishes contracts httpgen cannot implement.

protoc-gen-openapiv3 handles combinations httpgen rejects. Given a message with both nullable and timestamp_format:

Both:
  type: object
  properties:
    f1: { type: [string, "null"] }                                   # nullable, honored
    f2: { type: integer, format: unix-timestamp,
          description: Unix timestamp in seconds }                   # timestamp_format, honored

That spec is correct and complete. The Go server for the same proto does not compile. We ship an API description that no sebuf-generated server can satisfy — the spec is the artifact consumers integrate against, so the divergence is not cosmetic.

2. Nested annotations silently emit spec-violating bytes.

The pipeline starts from opts.Marshal(x), and protojson knows nothing about a child's MarshalJSONSebuf. So a child's annotation is dropped whenever the parent has a marshaler of its own:

message Inner { bytes b = 1 [(sebuf.http.bytes_encoding) = BYTES_ENCODING_HEX]; }
message Outer {
  Inner inner = 1;
  optional string n = 2 [(sebuf.http.nullable) = true];
}
Inner marshaled alone:     {"b":"48656c6c6f"}              ← hex, correct
Inner marshaled via Outer: {"inner":{"b":"SGVsbG8="}}      ← base64, annotation dropped

This cannot be fixed by hooking into protojson: protojson encodes purely through protoreflect, with a closed registry of well-known-type handlers keyed by full name (wellKnownTypeMarshaler in encoding/protojson/encode.go). It offers no user extension point and will never call a generated method. Delegation therefore has to be an explicit stage that we emit — which is precisely the stage that is missing today.

No error, no guard, no conflict reported. And OpenAPI describes that field as:

b: { type: string, pattern: ^[0-9a-fA-F]*$, format: hex }

The server emits SGVsbG8=, which fails the spec's own pattern. Any spec-validating client rejects a response from our own server. This is wrong data on the wire today, not a missing feature.

3. The workarounds for this gap are the reason the feature set is tangled.

Three features hit the missing delegation step and each improvised differently:

  • nestsInt64NumberMessage — declared it a conflict and refuses to generate
  • flatten — hand-rolled child delegation inline
  • unwrap — abandoned the shared pipeline and hand-rolls the entire output map

One missing stage, three incompatible responses. That is also why unwrap collides with all eight other features and appears in no conflict list.

4. Ordinary API shapes are impossible.

#251's motivating case is string-mapped enums plus a nullable scalar on one message — {"type": "call", "delta": null}. That is a routine payload, not an exotic one. Today it cannot be expressed.

5. The failure mode is unpredictable.

Four different outcomes for what looks to a user like the same thing: some pairs generate correctly, some fail generation with a clear error, 19 of 36 generate and then break go build (#252), and nested cases silently corrupt output. Nothing in the annotation docs predicts which bucket you land in.

6. Extending the system is a landmine.

Adding a tenth feature today means: a new detector, edits to four hand-maintained conflict lists, a filename that must not clash with clientgen (#236), and an UnmarshalJSONSebuf twin (#235) — with nothing failing if any is forgotten. #235, #236, and #252 are three filed symptoms of that one structural gap.

Describe the solution you'd like

One marshaler per message, emitted once, composed from four ordered stages:

func (x *Msg) MarshalJSONSebuf(opts protojson.MarshalOptions) ([]byte, error) {
    if x == nil { return []byte("null"), nil }

    // 1. BASE — protojson does the unannotated majority
    data, err := opts.Marshal(x)
    if err != nil { return nil, err }
    var raw map[string]json.RawMessage
    if err := json.Unmarshal(data, &raw); err != nil { return nil, err }

    // 2. DELEGATE — re-encode message-typed fields whose type has its own
    //    marshaler, so child annotations survive. Fixes "why this matters" #2.
    if m, ok := any(x.Inner).(sebufMarshaler); ok {
        if raw["inner"], err = m.MarshalJSONSebuf(opts); err != nil { return nil, err }
    }

    // 3. FIELD TRANSFORMS — one block per annotated field. Disjoint keys,
    //    order-independent. All eight field-local features live here.
    if x.F1 == nil { raw["f1"] = []byte("null") }               // nullable
    raw["f2"], _ = json.Marshal(x.F2.AsTime().Unix())           // timestamp_format

    // 4. DOCUMENT TRANSFORM — root unwrap only; replaces the object. Runs last.
    return json.Marshal(raw)
}

Three consequences worth calling out:

The conflict machinery is deleted, not improved. Stage 3 mutations touch only the key of the field they annotate, so they compose by construction. The single genuinely exclusive thing is stage 4, and root unwrap is already a per-message property (annotations.IsRootUnwrap), so at-most-one is structural rather than validated. The four hand-maintained conflict lists and their detectors go away.

unwrap stops being special once split in two. Map-value unwrap rewrites one key's value — that is a stage 3 field transform. Only root unwrap replaces the document (stage 4). Making that distinction is what lets unwrap rejoin the pipeline instead of hand-rolling every sibling field, which is also what makes it silently drop sibling annotations today.

UnmarshalJSONSebuf gets the same treatment. One unmarshaler per message composing each transform's inverse, which closes #235 structurally rather than by remembering to write five more methods.

Describe alternatives you've considered

Harden the exclusion instead — a single marshalJSONFeatures registry replacing the four hand-maintained lists, so every unsupported pair fails generation cleanly. This was filed as #253 and has been closed in favor of this issue. It fixes #252's broken builds, but it is more code written to preserve an accident, it leaves #251 permanently unfixable, and it does nothing about the nested-drop corruption in "why this matters" #2 — which is the most damaging of the three, since it produces wrong bytes with no diagnostic at all.

Do nothing / document the restriction — the nested-drop bug makes this untenable. It is a correctness defect against our own published OpenAPI spec, independent of ergonomics.

Additional context

Suggested sequencing, cheapest and most valuable first:

  1. Stage 2 alone (nested delegation). Independently valuable, fixes the silent corruption, deletes nestsInt64NumberMessage, and is a prerequisite for stages 3 and 4 being correct. Ships without touching the exclusion model at all.
  2. Merge the eight pipeline emitters into one stage-3 emitter. Delete the conflict lists. Closes httpgen: MarshalJSON conflict guards miss 19 of 36 feature pairs, emitting uncompilable Go instead of failing generation #252 and [BUG] Can't combine enum_value with empty_behavior #251.
  3. Port unwrap — split map-value (stage 3) from root (stage 4). The only real rewrite.
  4. Mirror for UnmarshalJSONSebuf — closes [BUG] httpgen: five emitters still lack the UnmarshalJSONSebuf twin, so DiscardUnknown is dropped #235.

Safety invariant for the migration: single-feature messages must be byte-identical before and after. Golden files will churn broadly; if that invariant holds, the churn is noise and the only behavior changes are (a) combinations that previously failed and (b) nested annotations that previously corrupted. An all-pairs generate-and-compile test over the 36 combinations, in the spirit of internal/urlparamtest, is the natural regression net.

Scope is httpgen. protoc-gen-go-client duplicates much of this surface (#236) and should be evaluated for sharing the composed emitter rather than growing a parallel copy.

Supersedes #253. Implements #251. Closes #252 and #235 structurally.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or requestfoundationInfrastructure, refactoring, bug fixesgen/go-httpprotoc-gen-go-http (HTTP server)json-mappingJSON serialization mapping features

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions