Skip to content

Preserve length limits in body parameter definitions - #1011

Open
numbata wants to merge 4 commits into
masterfrom
fix/body-param-length-limits
Open

numbata wants to merge 4 commits into
masterfrom
fix/body-param-length-limits

Conversation

@numbata

@numbata numbata commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Length limits that API authors declare on JSON body fields, such as a name capped at 50 characters or a list capped at three entries, were missing from the generated Swagger document, so consumers could not see the rules the API actually enforces. This change publishes those limits for string and array fields in request bodies, including fields inside nested objects and arrays of objects. API owners do not need to change any declarations; the limits appear after regenerating the documentation.

Root cause

grape-swagger already extracts the limits correctly. When a Grape param declares length: { min:, max: }, ParseParams emits minLength / maxLength for strings and minItems / maxItems for arrays. For POST, PUT and PATCH, MoveParams then moves body params out of parameters and rebuilds them as a schema under definitions. That rebuild copies keys through an allowlist and silently drops everything not on it, and the four length keys were not on it. Query and path params never go through this step, which is why only body fields lost their limits. Fixes #1010.

What changes

  • minLength and maxLength are added to the keys copied onto string properties in a body definition.
  • minItems and maxItems are copied onto the array schema itself, where they describe the number of entries. They are not copied onto items, so an item-count limit never turns into a per-string character limit.
  • Explicit zero minimums (min: 0) are kept; the keys are copied by presence, not truthiness.
  • Nested objects and arrays of objects (Array[JSON] with a block) get the same treatment, because they go through the same conversion.
  • The array-specific keys are kept in a named array_schema_keys method next to property_keys, so both allowlists are visible in one place.
  • Fields without a length constraint keep exactly their previous schema shape.

Benefits

  • The published contract matches what Grape validates, so developers no longer discover limits through failed requests.
  • Tools that build forms, client-side validation or generated clients from the schema get the information they need.
  • The limits come from the existing length: declarations, so there is a single source of truth and nothing to repeat in documentation: options.
  • The diff is additive and small, and it is trivially revertible.

Drawbacks and nuances

  • Generated documents change. Anything diffing or snapshotting the Swagger JSON, and any client regenerated from it, will show new minLength / maxLength / minItems / maxItems entries. Schema-aware tools may begin enforcing limits that were previously absent from the document, and how they do so is up to each tool. API owners should review regenerated artifacts and schema snapshots when upgrading.
  • This only changes documentation. Grape still validates requests, and no new server-side rules are added.
  • The scope is these four keys. Other keywords that ParseParams may add in the future (for example pattern or uniqueItems) still need to be added to the allowlists by hand. This PR makes those lists easier to find but does not remove the need to maintain them.
  • If a body param's type resolves to a definition, the converter writes $ref and, with this change, also any length keys on the same object. Swagger 2.0 ignores siblings of $ref, so those keys have no effect there. Putting length: on an entity-typed param is unusual, and description already behaves the same way.
  • The specs cover POST, PUT and PATCH. DELETE is also in move_methods, but DELETE bodies are rare and are not exercised.

Alternatives considered

  • Pass every key through instead of using an allowlist. This would prevent this class of bug, but body definitions would then also receive Grape-internal and non-schema keys (in, name, required, and so on), changing output well beyond the four limits. It is a larger behavior change than this fix warrants.
  • Share one key list between the property and array paths. The two paths intentionally copy different keys, because array limits belong on the array and element keys belong on items. Merging them would risk putting item counts on items or character limits on the array. Keeping the array keys in a named method next to the property keys keeps the distinction while making both lists easy to find.
  • Ask API authors to repeat limits through documentation: options. This duplicates information that is already declared and would leave every existing API with the gap.

Length limits were parsed but omitted when body parameters became schema properties. Copy string and array bounds during conversion and cover POST, PUT, PATCH, nested objects, and zero minimums with regression tests.
@github-actions

github-actions Bot commented Oct 26, 2026 •

Copy link
Copy Markdown

Danger Report

No issues found.

View run

The array path in MoveParams kept its own inline key list next to the shared
property_keys, so a new keyword has to be remembered in two spots. Naming the
array keys puts both lists side by side. The specs now cover arrays of objects
and assert the nested properties exactly.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@numbata
numbata requested a balanced review from Copilot October 26, 2026 11:05

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

The keys belong to the array schema itself, while property_keys is also used to build the array's items, so the old name suggested the opposite split.
@numbata
numbata marked this pull request as ready for review October 26, 2026 11:46
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.

Length limits (minLength/maxLength, minItems/maxItems) are dropped from body params moved into a definition

2 participants