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

Improved type safety tests #494

Merged
merged 9 commits into from
Jul 11, 2019
Merged

Conversation

ilammy
Copy link
Collaborator

@ilammy ilammy commented Jul 9, 2019

JavaScript is dynamically typed, therefore it is highly likely that at some point one of our users will try using something that's not supposed to be used with Themis functions. Add some more tests to ensure that we do cover all the bases and do not accept invalid arguments when we do not support them.

There are quite a few cases that we need to cover:

  • null & undefined (but sometimes null is allowed)
  • strings (that's debatable, but most API should not accept them directly)
  • untyped JavaScript arrays as well as all typed arrays that are not byte arrays
  • promises which could come from other async API in JavaScript
  • random objects should not be implicitly casted into arrays (one never knows...)

I don't want to miss any of these by chance, and it will be nice to be able to add or remove exceptions for all tests simultaneously. I have added a helper functions to enumerate all possible combinations for arguments. Hopefully it should make it easier to write combinatorial tests.


Add a utility function which calls the provided closure for all possible
combination of arguments from the provided arrays. This function allows
testing multiple combinations of various invalid arguments and ensure
that we don't miss any of them. Enjoy your dynamic typing experience.
Add tests to verify that key generation functions do not accept any
generally invalid arguments, such as "null", "undefined", etc. We
accept only byte arrays. No strings, no integer arrays, no promises
of arrays. Just Monika^W bytes.
@ilammy ilammy added the W-WasmThemis 🌐 Wrapper: WasmThemis, JavaScript API, npm packages label Jul 9, 2019
@ilammy
Copy link
Collaborator Author

ilammy commented Jul 9, 2019

Currently this covers only key generation. I'll add tests for other cryptosystems into this PR once they are merged.

Ensure that we disallow any unexpected types in Secure Cell input.

There are a few exceptions from the generally invalid types:

  - Secure Cell accepts "null" values for context in Seal and
    Token Protect modes. But not for other arguments, and not
    in the Context Imprint mode.

  - Context Imprint mode has a custom error for "undefined" context
    value which is possible if the user omits the context argument
    (mistakingly assuming that it is optional)

In all other cases we expect a TypeError to be thrown.
Ensure that we disallow any unexpected types in Secure Message input.
The expectations are regular. However, in Secure Message constructor
we produce Themis errors instead of TypeErrors.
Ensure that we disallow any unexpected types in Secure Comparator.
Ensure that we disallow any unexpected types in Secure Session.
Actually, keys cannot be constructed from any string at the moment,
but let's test this specifically because we rely on PrivateKey and
PublicKey to contain valid non-empty keys at all times.
@ilammy
Copy link
Collaborator Author

ilammy commented Jul 9, 2019

I've added type checks for all cryptosystems. Hopefully, I did not miss any API entry point.

@vixentael, there's also an additional check for empty strings when constructing keys.

It turned out that I did not actually need that combinatorial hack
so replace it with a more straightforward forEach loop. Well, that
was a nice exercise.
Copy link
Contributor

@vixentael vixentael left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool! i'd like to prevent js users from making mistakes and mis-using API.

@ilammy ilammy merged commit a58e0c8 into cossacklabs:master Jul 11, 2019
@ilammy ilammy deleted the wasm-type-safety branch July 26, 2019 13:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
W-WasmThemis 🌐 Wrapper: WasmThemis, JavaScript API, npm packages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants