Skip to content

Safely access schema to prevent crashes #881

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
+5 −5
Open

Safely access schema to prevent crashes #881

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1306226
Safely access schema to prevent crashes
makrandgupta Jun 24, 2025
16fe863
Add fallback for security scheme
makrandgupta Jun 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions lib/spec/openapi/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ function plainJsonObjectToOpenapi3 (container, jsonSchema, externalSchemas, secu
}

// description should be optional
if (jsonSchemaElement.description) result.description = jsonSchemaElement.description
if (jsonSchemaElement?.description) result.description = jsonSchemaElement.description
Copy link

Copilot AI Nov 6, 2025

Choose a reason for hiding this comment

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

Inconsistent defensive programming: optional chaining is used in the condition check, but jsonSchemaElement.description is accessed directly in the assignment. If jsonSchemaElement could be null/undefined, the assignment should also use optional chaining: result.description = jsonSchemaElement?.description. However, this defensive check appears unnecessary since jsonSchemaElement is always passed as a parameter from line 194 where it's accessed as obj[propKey], suggesting it should always be defined. Consider removing the optional chaining from the condition or applying it consistently.

Suggested change
if (jsonSchemaElement?.description) result.description = jsonSchemaElement.description
if (jsonSchemaElement.description) result.description = jsonSchemaElement.description

Copilot uses AI. Check for mistakes.
return result
}
break
Expand Down Expand Up @@ -239,16 +239,16 @@ function resolveSchemaExamplesRecursive (schema) {
function schemaToMedia (schema) {
const media = { schema }

if (schema.examples?.length === 1) {
if (schema?.examples?.length === 1) {
media.example = schema.examples[0]
delete schema.examples
} else if (schema.examples?.length > 1) {
} else if (schema?.examples?.length > 1) {
media.examples = convertExamplesArrayToObject(schema.examples)
// examples is invalid property of media object schema
delete schema.examples
}

if (schema[xExamples]) {
if (schema && schema[xExamples]) {
media.examples = schema[xExamples]
delete schema[xExamples]
Comment on lines +251 to 253
Copy link

Copilot AI Nov 6, 2025

Choose a reason for hiding this comment

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

Inconsistent null-checking pattern: This change uses explicit schema && schema[xExamples] while the surrounding code on lines 242 and 245 uses optional chaining (schema?.examples). For consistency and readability, this should also use optional chaining: if (schema?.[xExamples]). However, the deeper issue is that if schema could be null/undefined, lines 252-253 would fail when accessing schema[xExamples] directly without optional chaining.

Suggested change
if (schema && schema[xExamples]) {
media.examples = schema[xExamples]
delete schema[xExamples]
if (schema?.[xExamples]) {
media.examples = schema?.[xExamples]
delete schema?.[xExamples]

Copilot uses AI. Check for mistakes.
}
Expand Down Expand Up @@ -451,7 +451,7 @@ function prepareOpenapiMethod (schema, ref, openapiObject, url) {
]
.reduce((acc, securitySchemeGroup) => {
Object.keys(securitySchemeGroup).forEach((securitySchemeLabel) => {
const scheme = openapiObject.components.securitySchemes[securitySchemeLabel]
const scheme = openapiObject.components.securitySchemes[securitySchemeLabel] ?? {}
const isBearer = scheme.type === 'http' && scheme.scheme === 'bearer'
const category = isBearer ? 'header' : scheme.in
const name = isBearer ? 'authorization' : scheme.name
Expand Down