Skip to content

Commit

Permalink
[fix] Unexpected null request bodies result in 400 response status (#248
Browse files Browse the repository at this point in the history
)

Previously this resulted in a `SafeNullPointerException`, which mapped
to a 500 response status.
  • Loading branch information
carterkozak authored and bulldozer-bot[bot] committed Feb 25, 2019
1 parent 902a33e commit 7dcfb16
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,21 @@ public void java_url_client_receives_bad_request_without_authheader() throws IOE
assertThat(httpUrlConnection.getResponseCode()).isEqualTo(400);
}

@Test
public void java_url_client_receives_bad_request_with_json_null() throws IOException {
HttpURLConnection connection = preparePostRequest();
connection.setRequestProperty("Authorization", "Bearer authheader");
connection.setRequestProperty(HttpHeaders.CONTENT_TYPE, "application/json");
connection.setRequestProperty(HttpHeaders.ACCEPT, "application/json");
sendPostRequestData(connection, "null");
assertThat(connection.getResponseCode()).isEqualTo(400);
try (InputStream responseBody = connection.getErrorStream()) {
SerializableError error = CLIENT_OBJECT_MAPPER.readValue(responseBody, SerializableError.class);
assertThat(error.errorCode()).isEqualTo("INVALID_ARGUMENT");
assertThat(error.errorName()).isEqualTo("Default:InvalidArgument");
}
}

@Test
public void java_url_client_receives_unprocessable_entity_with_null_body() throws IOException {
HttpURLConnection httpUrlConnection = preparePostRequest();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,8 @@ public void serialize(Object value, OutputStream output) throws IOException {
public final <T> T deserialize(InputStream input, TypeToken<T> type) throws IOException {
try {
T value = mapper.readValue(input, mapper.constructType(type.getType()));
Preconditions.checkNotNull(value, "cannot deserialize a JSON null value");
// Bad input should result in a 4XX response status, throw IAE rather than NPE.
Preconditions.checkArgument(value != null, "cannot deserialize a JSON null value");
return value;
} catch (MismatchedInputException e) {
throw FrameworkException.unprocessableEntity("Failed to deserialize response stream. Syntax error?",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import static org.mockito.Mockito.verify;

import com.google.common.reflect.TypeToken;
import com.palantir.logsafe.exceptions.SafeIllegalArgumentException;
import com.palantir.logsafe.exceptions.SafeNullPointerException;
import java.io.ByteArrayInputStream;
import java.io.IOException;
Expand Down Expand Up @@ -57,7 +58,7 @@ public void json_serialize_rejectsNulls() {
public void json_deserialize_rejectsNulls() throws IOException {
// TODO(rfink): Do we need to test this for all primitive types?
assertThatThrownBy(() -> json.deserialize(asStream("null"), new TypeToken<String>() {}))
.isInstanceOf(SafeNullPointerException.class);
.isInstanceOf(SafeIllegalArgumentException.class);
assertThat(json.deserialize(asStream("null"), new TypeToken<Optional<String>>() {})).isEmpty();
}

Expand Down

0 comments on commit 7dcfb16

Please sign in to comment.