TypeError: Bad input string - Hash.update

[nathanielescribano]

In index.js

  var hash = escape(crypto
    .createHash('sha1')
    .update(salt)
    .update('-')
    .update(secret)
    .digest('base64'))
  return salt + '-' + hash

it looks like .update("-") causes a TypeError: Bad input string error. It seems to work find when i remove that line. I'm wondering if it's an issue with an older version of crypto? since it seems to work find when i pull up a node console. Any thoughts?

[dougwilson]

Hmm, not sure. What version of Node.js is the error occurring on? Any way I can reproduce? The versions running on Travis CI are not having the issue.

[nathanielescribano]

I'm using 0.12.1. npm - 2.5.1 whats interesting is if i open up a node console and run c.createHash('sha1').update("salt").update("-").digest("base64") it works fine. But when it runs within my app, i get the issue described above.

[dougwilson]

Gotcha. I'm not able to reproduce on 0.12.1. Anyway you can help me? What about can you paste a complete file I can paste on my system and run that will throw that error?

[nathanielescribano]

Figured out the issue. I was using https://github.com/jas-/connect-redis-crypto/blob/master/lib/connect-redis.js This file has a line crypto.DEFAULT_ENCODING = 'hex' . That leaves the DEFAULT_ENCODING to 'hex' so when csurf calls .update('-') it crashes.

in a node console doing

crypto.DEFAULT_ENCODING = "hex" 
crypto.createHash('sha1').update('salt').update('-')

gives me the error I mentioned earlier.

seems like one shouldn't change the DEFAULT_ENCODING as it might affect other modules, but should csurf be explicit about what encoding needs to be used? Any general thoughts on this?

[nathanielescribano]

Thanks a lot for being so responsive by the way :dancer:

[dougwilson]

Ah ha! Yes, I agree on both points: no, a module should not be doing that and yes, we should be explicit, especially since it won't hurt anything. Feel free to submit a PR, otherwise I'll get around to it eventually :)

[dougwilson]

Ok, I'm going to add the explicitness now :) I also noticed that the above crypto is not actually a part of this module, but rather a part of a dependency, so I'll be updating the csrf module and closing this issue :)

[dougwilson]

Ok, version 1.8.1 will be published shortly with the fix :)

[nathanielescribano]

awesome! Thanks!!