I recently ran into a problem where the callers of one of my methods was using it improperly by suppling a parameter that did not conform to the required values. However, the problems did not surface until I changed the implementation; previously, things worked by chance due to an implementation detail.
I had been using input validation, but my validation procedure was not stringent enough to detect the improper parameter. This was tied to the fact that before the implementation change, things worked perfectly fine even with the improper inputs. However, the implementation change “broke” this work flow. All of this has led me to believe in the great importance of:
- Draconian validation: For inputs, only accept what you say you will accept. Nothing else should pass.
- Proper unit testing: Test your method with a variety of inputs, both expected and unexpected or non-conforming. You never know what callers will pass into your methods.
Let me go into some more detail.
I had written a method that would accept a FQDN (Fully-Qualified Domain Name) as its argument. The purpose of this method was to retrieve some information over HTTPs from the web server running on the server with the specified domain. Since domain names are protocol-neutral (i.e. they aren’t preceded by something like “http://”), I ended up forming a
URL object based on the FQDN, since eventually I’d be retrieving information over HTTPs anyways:
final URL url = new URL("https://" + fqdn + "/path_to_service");
I figured that this was enough protection in terms of input validation, since the constructor for
URL throws a
MalformedURLException if something goes wrong. (There was other code to check if the web server at the specified URL could actually be contacted and this was contained in the actual retrieval)
However, it turned out that callers had started using this method by passing in not just an FQDN, but an FQDN followed by a port number in the following format: host.name:portNumber. This was being used so that servers operating on non-default HTTPs ports could be contacted.
Initially, this worked, as then the statement above was executed with something like this:
// With fqdn == "some.host.name:portNumber", nothing fails. final URL url = new URL("https://" + fqdn + "/path_to_service");
This was because the URL formed was still valid. However, this was an invalid use of my method because we had never specified that this sort of input was allowed. It just happened to work. The issue of non-default HTTPs ports and the usage here was never discussed and because it seemingly worked, nothing was done about it.
Some time went on until I decided to make an implementation change. Instead of retrieving the information from the web server itself, I would instead retrieve it directly from the TLS handshake process using a API provided to me. (The information was related to the certificates)
This API accepted two parameters: The server host name or FQDN, and a separate port number. Not realizing that callers were passing in non-default port numbers in the first place, I ignorantly just passed the FQDN supplied to me into this API along with the default HTTPs port. (443)
As expected, this broke the functionality that callers had come to expect of my API, namely, custom HTTPs ports specified alongside the FQDN in the argument. It was only after these complaints were filed that I realized my API was expected to take not just the FQDN, but something like “server.host.name:8080”.
The problem was quickly fixed as the solution was fairly trivial, but the causes have taught me several lessons.
Be Draconian in your input validation
Or, don’t trust your callers to read your documentation. Initially, I thought my method was to accept only an FQDN. Even though I was wrong in this respect, the problem could’ve been identified earlier if I had coded my method to fail (i.e. throw an exception) if anything that was not an FQDN was passed in. (Simply forming a
URLobject around it was not stringent enough) This way, things wouldn’t have initially worked by coincidence and I would’ve had an earlier opportunity to correct my mistake.
Unit test for exceptional cases
My unit testing of my method was not extensive enough. Though I coded both pass and fail tests using JUnit, I did not test for cases where someone would (very likely) try to pass in a port number along with the FQDN. This step goes hand-in-hand with the above: Code your methods to accept only what you’ve said they’d accept, and fail on everything else. Then, unit test to make sure this contract is fulfilled.
While the problem was frustrating, I’m glad to have learned these lessons.