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.
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
Header name is case insensitive in the schema definition #816
Conversation
Unfortunately the schema this fix only for jsonschema and only if the schema is a plain object.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
lib/validation.js
Outdated
@@ -29,7 +29,15 @@ function build (context, compile, schemas) { | |||
context.schema = schemas.resolveRefs(context.schema) | |||
|
|||
if (context.schema.headers) { | |||
context[headersSchema] = compile(context.schema.headers) | |||
// see https://github.com/fastify/fastify/pull/816 | |||
const headersSchemaLowerCase = {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you please link to the spec?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
lib/validation.js
Outdated
// https://tools.ietf.org/html/rfc2616#section-4.2 | ||
const headersSchemaLowerCase = {} | ||
for (const k in context.schema.headers) { | ||
headersSchemaLowerCase[k] = context.schema.headers[k] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should probably use hasOwnProperty
here, or switch to Object.keys()
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with @mcollina's suggestion.
lib/validation.js
Outdated
} | ||
if (context.schema.headers.properties) { | ||
for (const k in context.schema.headers.properties) { | ||
headersSchemaLowerCase.properties[k.toLowerCase()] = context.schema.headers.properties[k] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, thanks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
As titled, the rfc https://tools.ietf.org/html/rfc2616#section-4.2 defines the header key as case insensitive.
The problem is when I curl with
uppercase: 3
header because nodejs force to use to lower case.I'm working on to fix this.
Checklist
npm run test
andnpm run benchmark