search

rolandweber / Pityoulish

A collection of programming/debugging exercises to support a course on Distributed Systems.
Creative Commons Zero v1.0 Universal
7 stars 0 forks source link

Explicit input validation on the server #11

Closed rolandweber closed 7 years ago

rolandweber commented 8 years ago

Input data sent by the client should be validated explicitly: Text, Originator, Marker, Ticket. The initial implementation of the Message Board server with sockets just passes these values to server-side APIs and relies on implicit validation of arguments. Except for the Limit, which is already validated explicitly because the binary protocol specifies the valid range.

Implement explicit validation logic that can be re-used for Java RMI as well. Re-usable logic cannot throw a ProtocolException, because that class is specific to the Sockets implementation. Actual validation can take place in the generic server-side code (pityoulish.msgboard and pityoulish.tickets), but there should be an API which is independent of those. This allows to enforce additional constraints in the protocol layer.

rolandweber commented 8 years ago

Funny side effect: The ticket manager does not check the characters in the username. You can obtain tickets for username " ", for example. Of course, the ticket will also contain the whitespace from the username.

rolandweber commented 8 years ago

The initial implementation of the Message Board server with Java RMI will also rely on implicit validation of arguments. To be fixed with this issue as well.

rolandweber commented 7 years ago

How to handle null checks? Should these be part of the sanity check, and return a problem description? Or should the checks only be called for non-null arguments?

As of now, the DefaultMSanityChecker and DefaultTSanityChecker are inconsistent in this regard. They throw NullPointerException for missing mandatory arguments, instead of returning an error description. But DefaultMSanityChecker tolerates a null marker, because that's an optional argument.

It might be better to leave it to the caller whether an argument is optional. Then it would make sense for the checkers to require non-null arguments and throw exceptions for null. Or to report a missing argument as invalid.

rolandweber commented 7 years ago

As for optional arguments, the caller has to handle that. Sanity checkers will complain about null arguments. I'm still not sure whether I should throw an exception or report as missing. For now, an exception is thrown.

rolandweber commented 7 years ago

It's good enough for now. See #53 and #55 for what else could be done in this area. Help is welcome!