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

Be more explicit about S.get(s) being an escape hatch from type safety #708

Open
Avaq opened this issue Mar 1, 2021 · 1 comment
Open

Comments

@Avaq
Copy link
Member

Avaq commented Mar 1, 2021

I've run into a bug of the following shape a few times now, enough to stand out to me as a problem.

const input = {
  powerLevel: S.head ([9999])
};

// The developer misunderstands the shape of `input`:
const powerLevel = S.get (S.is (Number)) ('powerLevel') (input)

const phrase = S.maybe ("We don't know the power level")
                       (x => x > 9000 ? "It's over 9000!" : "No threat")

console.log (phrase)

The above code will log We don't know the power level every time, because S.is (Number) will return false for Maybe Number.

This problem also affects S.gets, and to a lesser extent S.parseJson. Because they don't perform any type checking on their input anything you throw at them will become "valid" output (even if that means always getting Nothing). Another way to put it is that there no way to distinguish between expected and unexpected "garbage" inputs. When I give S.get expected garbage, I get a clean Nothing, but when I give it actual garbage, I get a garbage Nothing.

I don't see a clear way around this. The program has no way to know which garbage is the result of the untrusted source, and which garbage came as the result of a developer mistake. But maybe there is something Sanctuary could do to prevent developers from falling into this trap?

Perhaps just make it more clear in the documentation for these functions that you need to be careful with them?
Or use a more conspicuous name than get? Something like unsafeGet, getUntrusted, ...? Something to trigger the developer or reviewer to think twice and tread lightly.

@davidchambers
Copy link
Member

Perhaps just make it more clear in the documentation for these functions that you need to be careful with them?

I like this idea. We could draw attention to the presence of Any in the type signature, and state that S.get should only be used when consuming external input.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants