Skip to content

fix(openapiv2): Invalid entries in body parameter schema required array when using body: "field_name"#6088

Merged
johanbrandhorst merged 2 commits intogrpc-ecosystem:mainfrom
rdark:fix-body-parameter-required-array
Nov 12, 2025
Merged

fix(openapiv2): Invalid entries in body parameter schema required array when using body: "field_name"#6088
johanbrandhorst merged 2 commits intogrpc-ecosystem:mainfrom
rdark:fix-body-parameter-required-array

Conversation

@rdark
Copy link
Copy Markdown
Contributor

@rdark rdark commented Nov 9, 2025

Summary

When using body: "field_name" in HTTP annotations with path parameters extracted from nested fields, the body field name is incorrectly added to the schema's required array, causing either duplicate entries or invalid references to non-existent properties.

Bug 1: Duplicate Required Field

When the body field name matches a property in the nested message:

Proto definition:

message Comment {
  string name = 1;
  string comment = 2 [(google.api.field_behavior) = REQUIRED];
  string author = 3 [(google.api.field_behavior) = REQUIRED];
}

message UpdateCommentRequest {
  Comment comment = 1 [(google.api.field_behavior) = REQUIRED];
}

rpc UpdateComment(UpdateCommentRequest) returns (Comment) {
  option (google.api.http) = {
    patch: "/api/v1/{comment.name}"
    body: "comment"
  };
}

Generated OpenAPI (incorrect):

{
  "name": "comment",
  "in": "body",
  "required": true,
  "schema": {
    "required": ["comment", "author", "comment"]
  }
}

The "comment" field appears twice in the required array.

Expected:

{
  "name": "comment",
  "in": "body",
  "required": true,
  "schema": {
    "required": ["comment", "author"]
  }
}

Bug 2: Self-Referential Schema

When the body field name does NOT match any property in the nested message:

Proto definition:

message Direction {
  string name = 1;
  string title = 2 [(google.api.field_behavior) = REQUIRED];
}

message UpdateDirectionRequest {
  Direction direction = 1 [(google.api.field_behavior) = REQUIRED];
}

rpc UpdateDirection(UpdateDirectionRequest) returns (Direction) {
  option (google.api.http) = {
    patch: "/api/v1/{direction.name}"
    body: "direction"
  };
}

Generated OpenAPI (incorrect):

{
  "name": "direction",
  "in": "body",
  "required": true,
  "schema": {
    "properties": {
      "title": {"type": "string"}
    },
    "required": ["title", "direction"]
  }
}

The required array contains "direction", which is not a property of the schema.

Expected:

{
  "name": "direction",
  "in": "body",
  "required": true,
  "schema": {
    "properties": {
      "title": {"type": "string"}
    },
    "required": ["title"]
  }
}

Root Cause

When renderFieldAsDefinition is called for body parameters with path parameters, it calls updateSwaggerObjectFromFieldBehavior which adds the field name to the schema's required array if the field is marked REQUIRED.

However, the schema represents the field's type (the nested message), not the containing message. The body field name should not be in that schema's required array - the body parameter's required status is already correctly expressed via "required": true at the parameter level.

Trigger Conditions

This bug occurs when ALL of these conditions are met:

  1. HTTP annotation uses body: "field_name" (not body: "*")
  2. Path parameters are extracted from nested fields (e.g., {field_name.property})
  3. The field is marked [(google.api.field_behavior) = REQUIRED]

Environment

  • grpc-gateway version: v2.27.3 (and earlier versions)
  • protoc-gen-openapiv2: Latest

Additional Context

This is a separate issue from the nested required fields hoisting bug. That bug affects body: "*" while this bug affects body: "field_name".

References to other Issues or PRs

Have you read the Contributing Guidelines?

yes

@rdark rdark changed the title Bug: Invalid entries in body parameter schema required array when using body: "field_name" fix(openapiv2): Invalid entries in body parameter schema required array when using body: "field_name" Nov 9, 2025
Copy link
Copy Markdown
Collaborator

@johanbrandhorst johanbrandhorst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for another excellent analysis and PR, I suspect this will likely also cause some generation failures as it fixes existing generation bugs, if not, could you add a case of this to one of the example protos? Thanks!

…rray

When generating OpenAPI schemas for body parameters with body: "field_name"
and path parameters extracted from nested fields, the body field name was
incorrectly added to the schema's required array via updateSwaggerObjectFromFieldBehavior.

For example, with UpdateCommentRequest containing a required Comment field,
and Comment having a required "comment" field, when using path parameter
{comment.name} and body: "comment", the generated body schema incorrectly had:
  required: ["comment", "resource", "author", "comment"]
Or when the field name doesn't exist as a property:
  required: ["title", "direction"] where "direction" is not a property

This fix filters the required array after calling renderFieldAsDefinition to:
1. Remove duplicate entries of the body field name
2. Remove the body field name if it's not actually a property of the schema
The body parameter's required status is already correctly set via the
parameter's required: true attribute.
The fix in template.go correctly removes invalid body field names from the
required array when they are not properties of the schema. This regeneration
updates a_bit_of_everything.swagger.json to remove the incorrectly included
'book' field from the required array in the UpdateBookRequest body parameter.

Related to body parameter schema generation fix.
@rdark rdark force-pushed the fix-body-parameter-required-array branch from da2d098 to eb44769 Compare November 11, 2025 10:37
@rdark
Copy link
Copy Markdown
Contributor Author

rdark commented Nov 11, 2025

Thanks for another excellent analysis and PR, I suspect this will likely also cause some generation failures as it fixes existing generation bugs, if not, could you add a case of this to one of the example protos? Thanks!

I've rebased & updated examples for both PRs - I suspect that this one will have a conflict after merging #6078 so will need to rebase again and fix, but LMK!

Copy link
Copy Markdown
Collaborator

@johanbrandhorst johanbrandhorst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great

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.

2 participants