digitalbazaar / http-signature-header

BSD 3-Clause "New" or "Revised" License
8 stars 2 forks source link

Add string validation for sigString. #35

Closed mattcollier closed 3 years ago

mattcollier commented 3 years ago

Without validation, an error is produced when attempting to access sigString.length if sigString is undefined.

https://github.com/digitalbazaar/http-signature-header/blob/master/lib/index.js#L63

davidlehn commented 3 years ago

I was going to say this is ok since assert is used elsewhere already.

Since I was curious if browser testing would work given that assert hack, I added karma support: https://github.com/digitalbazaar/http-signature-header/pull/36

This got merged first, but I'd like to note that I think there are no tests that depend on the assert calls failing. I'm not sure if that is intentional or not. If that's just a dev feature that may be fine, but if it's supposed to be normal runtime checks, that's an issue. Also the use of Proxy in the browser assert hack will not play nice when targeting older browsers like IE11(?).