Skip to content

Commit

Permalink
Add a null check before reading GitHub response (#563)
Browse files Browse the repository at this point in the history
Add a null check to prevent NullPointer exception while reading body from the GitHub API response. response.body() should not return null according to the java documentation, but the NullPointer exception was found in the customer debug logs
  • Loading branch information
vinokurig authored Sep 21, 2023
1 parent a3cceea commit 261be8f
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,10 @@ private <T> T executeRequest(
} else if (response.statusCode() == HTTP_NO_CONTENT) {
return null;
} else {
String body = CharStreams.toString(new InputStreamReader(response.body(), Charsets.UTF_8));
String body =
response.body() == null
? "Unrecognised error"
: CharStreams.toString(new InputStreamReader(response.body(), Charsets.UTF_8));
switch (response.statusCode()) {
case HTTP_BAD_REQUEST:
throw new ScmBadRequestException(body);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
import static java.net.HttpURLConnection.HTTP_BAD_REQUEST;
import static java.net.HttpURLConnection.HTTP_NOT_FOUND;
import static java.net.HttpURLConnection.HTTP_NO_CONTENT;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.when;
import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertEqualsNoOrder;
import static org.testng.Assert.assertFalse;
Expand All @@ -33,10 +35,13 @@
import com.github.tomakehurst.wiremock.common.Slf4jNotifier;
import com.google.common.net.HttpHeaders;
import java.lang.reflect.Field;
import java.net.http.HttpClient;
import java.net.http.HttpResponse;
import org.eclipse.che.api.factory.server.scm.exception.ScmBadRequestException;
import org.eclipse.che.api.factory.server.scm.exception.ScmCommunicationException;
import org.eclipse.che.api.factory.server.scm.exception.ScmItemNotFoundException;
import org.eclipse.che.commons.lang.Pair;
import org.mockito.Mockito;
import org.mockito.testng.MockitoTestNGListener;
import org.testng.annotations.AfterMethod;
import org.testng.annotations.BeforeMethod;
Expand Down Expand Up @@ -260,6 +265,24 @@ public void shouldThrowExceptionOnNotFoundError() throws Exception {
client.getUser("token");
}

@Test(
expectedExceptions = ScmBadRequestException.class,
expectedExceptionsMessageRegExp = "Unrecognised error")
public void shouldThrowExceptionWithUnrecognisedErrorIfResponseBodyIsNull() throws Exception {
// given
HttpClient httpClient = Mockito.mock(HttpClient.class);
HttpResponse response = Mockito.mock(HttpResponse.class);
Field declaredField = GithubApiClient.class.getDeclaredField("httpClient");
declaredField.setAccessible(true);
declaredField.set(client, httpClient);
when(httpClient.send(any(), any())).thenReturn(response);
when(response.body()).thenReturn(null);
when(response.statusCode()).thenReturn(HTTP_BAD_REQUEST);

// when
client.getUser("token");
}

@Test(
expectedExceptions = ScmCommunicationException.class,
expectedExceptionsMessageRegExp =
Expand Down

0 comments on commit 261be8f

Please sign in to comment.