fix(cli): guard OpenAPI ingestion against pathologically long schema names#17099
fix(cli): guard OpenAPI ingestion against pathologically long schema names#17099devin-ai-integration[bot] wants to merge 2 commits into
Conversation
…names Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
![nitpickybot[bot]](https://avatars.githubusercontent.com/in/2901896?s=60&v=4){kind=small}
There was a problem hiding this comment.
AI Review Summary
Sensible defensive cap on pathologically long schema names, well-tested. One potential collision concern: the DJB2 hash is applied to the full token, but the cap only preserves the first 512 chars — two long tokens differing only after char 512 but sharing the same 32-bit hash would collide (astronomically unlikely, but worth noting). The implementation is otherwise clean.
- 🔵 1 suggestion(s)
| function hashToken(token: string): string { | ||
| let hash = 5381; | ||
| for (let i = 0; i < token.length; i++) { | ||
| hash = ((hash << 5) + hash + token.charCodeAt(i)) >>> 0; |
There was a problem hiding this comment.
🔵 suggestion
DJB2 truncated to 32 bits gives ~4B distinct values. For two schema names sharing a 512-char prefix and differing only in the tail, distinctness relies solely on this hash. Collision risk is negligible in practice, but if you want a stronger guarantee consider a 53-bit accumulation or appending the token length. Not blocking.
There was a problem hiding this comment.
Strengthened in 2af23d9: hashToken now combines two independent rolling hashes (different seeds) plus the token length into the suffix, widening the space well beyond 32 bits so two long tokens sharing a truncated prefix are extremely unlikely to collide. Still fully deterministic.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| if (token.length <= MAX_BREADCRUMB_TOKEN_LENGTH) { | ||
| return token; | ||
| } | ||
| return `${token.slice(0, MAX_BREADCRUMB_TOKEN_LENGTH)}_${hashToken(token)}`; |
There was a problem hiding this comment.
🟡 Token capping function is not idempotent, causing double-application to silently corrupt already-capped names
The capping function produces output longer than its own threshold (capBreadcrumbToken at packages/commons/core-utils/src/capBreadcrumbToken.ts:29) because 512 prefix + "_" + up to 7-char hash = 520 chars, which exceeds the 512-char limit, so a second application re-truncates and re-hashes the already-capped value.
Impact: Pathologically long schema names get a different generated type name than intended because the cap is applied twice (once in generateIr.ts:227 and again in getSchemaName.ts:7), producing an inconsistent identifier.
Double-capping mechanism and trigger condition
In packages/cli/api-importers/openapi/openapi-ir-parser/src/openapi/v3/generateIr.ts:227, the schema key is capped before being passed as a breadcrumb to convertSchema. Later, convertSchemaObject at packages/cli/api-importers/openapi/openapi-ir-parser/src/schema/convertSchemas.ts:382 calls getGeneratedTypeName(breadcrumbs, ...), which applies capBreadcrumbToken again at packages/cli/api-importers/openapi/openapi-ir-parser/src/schema/utils/getSchemaName.ts:7.
For a token of length > 512:
- First cap:
token.slice(0, 512) + "_" + hashToken(token)→ ~520 chars - Second cap sees 520 > 512, so:
cappedToken.slice(0, 512) + "_" + hashToken(cappedToken)→ different ~520 char result
The fix is to make the function idempotent by ensuring its output never exceeds the threshold. For example, slice to MAX - 1 - maxHashLength (e.g., 504) so the total output is always ≤ 512.
| return `${token.slice(0, MAX_BREADCRUMB_TOKEN_LENGTH)}_${hashToken(token)}`; | |
| return `${token.slice(0, MAX_BREADCRUMB_TOKEN_LENGTH - 8)}_${hashToken(token)}`; |
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Good catch — fixed in 2af23d9. capBreadcrumbToken now reserves room for the _<hash> suffix (prefixLength = MAX - hash.length - 1) so output is always ≤ MAX_BREADCRUMB_TOKEN_LENGTH, making it idempotent. Added a regression test asserting capBreadcrumbToken(capBreadcrumbToken(x)) === capBreadcrumbToken(x).
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Docs Generation Benchmark ResultsComparing PR branch against median of 5 nightly run(s) on
Docs generation runs |
SDK Generation Benchmark ResultsComparing PR branch against median of 5 nightly run(s) on Full benchmark table (click to expand)
main (generator): generator-only time via --skip-scripts (includes Docker image build, container startup, IR parsing, and code generation — this is the same Docker-based flow customers use via |
1 participant
Description
Some OpenAPI generators emit component-schema keys that are entire serialized type expressions (hundreds of KB each). A schema name is carried as a breadcrumb into every nested subschema's generated name and is repeatedly joined/
camelCase'd while walking the tree, so across thousands ofoneOf/allOfbranches a ~5 MB spec balloons into gigabytes and crashesfern generate --docswithFATAL ERROR: JavaScript heap out of memoryduring docs validation / OpenAPI→IR ingestion.Root cause is the upstream generator (schema keys should be short identifiers), but the importer had a robustness gap: it derives names from arbitrarily large schema keys with no bound. This adds a defensive cap at ingestion so a pathological name yields an ugly-but-valid identifier instead of taking the whole build down.
The fix caps the name at the source (where a schema name first enters the breadcrumbs) rather than only at the final name-generation step — otherwise the giant string keeps propagating through recursive breadcrumbs and is materialized in many downstream consumers.
The short base36 hash of the full token keeps distinct long names distinct and deterministic (so a
$refand its definition still resolve to the same generated name). Real schema/property names never approach 512 chars, so normal specs are unaffected and produce byte-for-byte identical output — nothing about docs rendering changes.Changes Made
capBreadcrumbTokento@fern-api/core-utils(shared, deterministic length cap + hash).openapi-ir-parser): cap the schema/response/parameter/property names extracted ingetBreadcrumbsFromReference, cap component-schema keys ingenerateIr, and cap tokens ingetGeneratedTypeNameas defense-in-depth.v3-importer-commons): cap breadcrumb tokens inconvertBreadcrumbsToName.Testing
capBreadcrumbToken(core-utils),getGeneratedTypeName, andgetBreadcrumbsFromReferencecap/determinism cases. Full suites pass: core-utils 119, openapi-ir-parser 93, v3-importer-commons 123.oneOf/ 1,127allOf):String::SlowFlatten→NewRawStringWithMap).--max-old-space-size=1024). Renaming the two giant keys to short identifiers produced the same ~92 MB / 0.4s, confirming the schema name — not the union nesting — was the cause.Link to Devin session: https://app.devin.ai/sessions/b411fdf7efcb4ec1ab0a6f84e34607af