FiloSottile / typage

A TypeScript implementation of the age file encryption format, based on libsodium.
BSD 3-Clause "New" or "Revised" License
64 stars 9 forks source link

chore: add property tests for encryption/decryption #6

Closed olastor closed 9 months ago

olastor commented 10 months ago

Hi, I created some basic property based tests using fast-check in case that's helpful for this project. The tests run a bit longer than the regular unit tests since they do multiple iterations with random data. With the current setting of 50 runs per property it takes ca. 1000ms on my machine. I think the scrypt factor adds the most delay, so I could also change it to use a slower number for these tests. Happy to make more contributions if there's something to do and I find the time.

olastor commented 10 months ago

More help is welcome, especially if there's something that the package is missing that I wouldn't know that would make it easier to use in more environments. (Like, does it bundle well for browsers? Should it have JSDocs?)

@FiloSottile A few minor things I noticed when reading the code superficially:

Regarding use in browser, I am not sure right now because I think sodium is using webasm. I think I could try to look into it, there are some packages like argon2-browser which might be good to take as examples. Docs of the different function parameters would be good to have, I think (even if it's just part of the README).

FiloSottile commented 9 months ago
  • I think you probably want to add "types": "./dist/index.d.ts" to package.json to publish the type declarations (see here).

Sweet, thanks.

  • The code often seems to use == and !=, but it's best to avoid those in favor of the type-safe equality operators ===/!== whenever possible. That's unintuitive when coming from other languages, but there are some crazy edge cases with the former operators.

Done, thank you for pointing to the ESLint rule, I remembered this being a thing but I wanted to have it enforced with a tool.

  • The way the classes/functions are exported seems a bit strange, but I guess that's mainly due sodium's ready function being called. I'll try to check if/how that could potentially be improved.

Yeah, this I went back and forth about a bunch. The issue is that libsodium is unusable (and unsafe) until the async sodium.ready is called. I wanted to make sure there was no way for a user to shoow themsleves in the foot by calling age functions before sodium.ready, and at the same time not make every age function async just because it might call sodium.ready. This is the best I could figure out, but I welcome suggestions, and it might need changing for browser bundling anyway.

Regarding use in browser, I am not sure right now because I think sodium is using webasm. I think I could try to look into it, there are some packages like argon2-browser which might be good to take as examples.

See #7.