Skip to content
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

[Bug]: lowercased webhook signature header not recognized by validateWebhook #399

Closed
thyming opened this issue Aug 5, 2023 · 3 comments · Fixed by #400
Closed

[Bug]: lowercased webhook signature header not recognized by validateWebhook #399

thyming opened this issue Aug 5, 2023 · 3 comments · Fixed by #400
Labels
triage Issue is being researched

Comments

@thyming
Copy link
Contributor

thyming commented Aug 5, 2023

Software Version

6.7.0

Language Version

Node 16

Operating System

Linux

What happened?

  1. Tried to validate a signature
  2. Library says it's not valid

What was expected?

validateWebhook is case-insensitive looking for the signature header.
fastify, for example, lowercases all incoming headers and the http spec says headers are case insensitive.

Sample Code

No response

Relevant logs

No response

@thyming thyming added the triage Issue is being researched label Aug 5, 2023
@Justintime50
Copy link
Member

Hey there, thanks for writing in! Can you help me understand the need for this? EasyPost should be sending these headers title cased as the code is already written. Are you saying you are receiving webhooks that have a lowercased header?

@thyming
Copy link
Contributor Author

thyming commented Aug 7, 2023

Yes, for example fastify lowercases all incoming headers.
Also, in http2, headers are explicitly required to be lowercased as part of the spec.
fastify/help#71

@Justintime50
Copy link
Member

Ah I see, your integration is using fastify so by the time your code gets the header, it's already been lowercased?

This is interesting. Your proposed solution should do the trick. We'll maybe want to revisit this holistically in the future but can move on this in the time being.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
triage Issue is being researched
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants