Skip to content

Separate logic from UI, secure logic with tests #1076

Closed
@andreasabel

Description

@andreasabel

Prompted by

Validators are working directly in the server monad, e.g.:

-- Make sure this roughly corresponds to the frontend validation in user-details-form.html.st
guardValidLookingEmail :: T.Text -> ServerPartE ()
guardValidLookingEmail str = either errBadEmail return $ do
guard (T.length str <= 100) ?! "Sorry, we didn't expect email addresses to be longer than 100 characters."
guard (T.all isPrint str) ?! "Unexpected character in email address, please use only printable Unicode characters."
guard hasAtSomewhere ?! "Oops, that doesn't look like an email address."
guard (T.all (not.isSpace) str) ?! "Oops, no spaces in email addresses please."
guard (T.all (not.isAngle) str) ?! "Please use just the email address, not \"name\" <person@example.com> style."

This makes it hard to test them cheaply.

There is a purely functional core that struggles to get out here: the actual validation does not need IO etc., it is pure logic.
Suggested restructuring:

  • data FooError = FooProblem1 | FooProblem2 | ...
  • checkFoo :: Foo -> Either FooError () does the actual check, indicating the violation by a Left FooProblem...
  • guardFoo :: Foo -> ServerPartE () calls checkFoo and handles the exception Left ... by escalating the problem to the user.
  • Add unit tests for checkFoo

Metadata

Metadata

Assignees

Labels

re: code qualityConcerning the code quality of the implementation of `hackage-server`

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions