Oliver works for a very large company. Just recently, someone decided that it was time to try out those “newfangled REST services.”
Since this was “new”, at least within the confines of the organization, that meant there were a lot more eyes on the project and a more thorough than average code review process. That’s how Oliver found this.
@SuppressWarning("null")
public void uploadData(Payload payload) {
// preparation of payload and httpClient
//...
@NonNull
HttpResponse response = null;
response = httpClient.execute(request);
if (response.getStatus() != 200) {
throw new RuntimeException(
String.format(
"Http request failed with status %s",
response.getStatus()
);
}
}
The purpose of this code is to upload a JSON-wrapped payload to one of the restful endpoints.
Let’s start with the error handling. Requiring a 200
status code is probably not a great idea. Hope the server side doesn’t say, “Oh, this creates a new resource, I had better return a 201
.” And you might be thinking to yourself, “Wait, shouldn’t any reasonable HTTP client raise an exception if the status code isn’t a success?” You’d be right, and that is what this httpClient
object does. So if the status isn’t a success code, the error throw
statement will never execute, meaning we only error if there isn't one.
The HttpResponse
variable is annotated with a @NonNull
annotation, which means at compile time, any attempt to set the variable to null should trigger an error. Which, you’ll note, is exactly what happens on the same line.
It’s okay, though, as the dependency setup in the build file was completely wrong, and they never loaded the library which enforced those annotations in the first place. So @NonNull
was just for decoration, and had absolutely no function anyway.