verify: does not gracefully handle invalid signatures about webhooks-methods.js

[gr2m]

why not both? Maybe start a PR with Deno tests, without removing the existing tests, and we take it from there?

[timrogers]

Thanks for reporting this!

Just to clarify, do the example values you provided in your issue trigger this error? (I haven't had a chance to test yet.)

Would you be open to making a PR to fix this? I'd be happy to review your contribution!

[mfulton26]

Thanks for reporting this!

Just to clarify, do the example values you provided in your issue trigger this error? (I haven't had a chance to test yet.)

Would you be open to making a PR to fix this? I'd be happy to review your contribution!

yes

I think any signature that starts with something other 2 or more non-hex characters will reproduce

[mfulton26]

I'll see if I have time to make a PR sometime today or tomorrow

[gr2m]

Looks like a Deno error to me, I cannot reproduce it with Node. Tested with Node v18.8.0 and v16.17.0.

Unfortunately the libraries are not native ESM yet, Deno is using a built version of the code base served from skypack.dev, the build step might also be the culprit here.

[mfulton26]

I see there are some browser tests. We can confirm if it is broken by adding additional test cases there first.

[mfulton26]

The browser version is completely independent of the node implementation. In fact, I think the browser version has some other behavior differences too. e.g. I have to strip "sha256=" from the signature before passing it into verify for the browser/web version too work.

[mfulton26]

It might be worthwhile to write the browser/web tests in Deno. Any concerns with me dropping puppeteer in favor of Deno here?

[mfulton26]

initial Deno tests PR: #81

[wolfy1339]

I think the proper thing to do is to catch these errors and handle them in your own code.

We are switching to outputting ESM in the next major version, which should help out with these things