Skip to content

Commit aac430e

Browse files
jenschudejenschude
committed
fix NPE when server response has no body
1 parent 3431113 commit aac430e

File tree

6 files changed

+108
-41
lines changed

6 files changed

+108
-41
lines changed

‎.yarn/install-state.gz‎

-25.8 KB
Binary file not shown.

‎commercetools/commercetools-sdk-java-api/src/main/java/com/commercetools/api/client/error/ApiHttpExceptionFactory.java‎

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11

22
package com.commercetools.api.client.error;
33

4+
import java.util.Optional;
5+
46
import io.vrap.rmf.base.client.ApiHttpException;
57
import io.vrap.rmf.base.client.ApiHttpRequest;
68
import io.vrap.rmf.base.client.ApiHttpResponse;
@@ -28,25 +30,26 @@ public ApiHttpException createClientException(ApiHttpRequest request, ApiHttpRes
2830
String message = "Client error response [url] " + request.getUri().toString() + " [status code] "
2931
+ response.getStatusCode() + " [reason phrase] " + response.getMessage();
3032

33+
final String body = Optional.ofNullable(response.getBody()).map(String::new).orElse("");
3134
switch (response.getStatusCode()) {
3235
case 400:
33-
return new BadRequestException(response.getStatusCode(), new String(response.getBody()),
34-
request.getHeaders(), message, response, request, serializer);
36+
return new BadRequestException(response.getStatusCode(), body, request.getHeaders(), message, response,
37+
request, serializer);
3538
case 401:
36-
return new UnauthorizedException(response.getStatusCode(), new String(response.getBody()),
37-
request.getHeaders(), message, response, request, serializer);
39+
return new UnauthorizedException(response.getStatusCode(), body, request.getHeaders(), message,
40+
response, request, serializer);
3841
case 403:
39-
return new ForbiddenException(response.getStatusCode(), new String(response.getBody()),
40-
request.getHeaders(), message, response, request, serializer);
42+
return new ForbiddenException(response.getStatusCode(), body, request.getHeaders(), message, response,
43+
request, serializer);
4144
case 404:
42-
return new NotFoundException(response.getStatusCode(), new String(response.getBody()),
43-
request.getHeaders(), message, response, request, serializer);
45+
return new NotFoundException(response.getStatusCode(), body, request.getHeaders(), message, response,
46+
request, serializer);
4447
case 409:
45-
return new ConcurrentModificationException(response.getStatusCode(), new String(response.getBody()),
46-
request.getHeaders(), message, response, request, serializer);
48+
return new ConcurrentModificationException(response.getStatusCode(), body, request.getHeaders(),
49+
message, response, request, serializer);
4750
}
48-
return new ApiClientException(response.getStatusCode(), new String(response.getBody()), response.getHeaders(),
49-
message, response, request);
51+
return new ApiClientException(response.getStatusCode(), body, response.getHeaders(), message, response,
52+
request);
5053
}
5154

5255
public static HttpExceptionFactory of(ResponseSerializer serializer) {

‎commercetools/commercetools-sdk-java-api/src/main/java/com/commercetools/api/client/error/ConcurrentModificationException.java‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11

22
package com.commercetools.api.client.error;
33

4+
import java.util.Optional;
5+
46
import com.commercetools.api.models.error.ConcurrentModificationError;
57
import com.commercetools.api.models.error.ErrorResponse;
68

@@ -57,11 +59,12 @@ public Long getCurrentVersion() {
5759
}
5860

5961
public static Long retrieveCurrentVersion(final ErrorResponse errorResponse) {
60-
return errorResponse.getErrors()
61-
.stream()
62-
.filter(errorObject -> errorObject instanceof ConcurrentModificationError)
63-
.findFirst()
64-
.map(errorObject -> ((ConcurrentModificationError) errorObject).getCurrentVersion())
62+
return Optional.ofNullable(errorResponse)
63+
.flatMap(response -> response.getErrors()
64+
.stream()
65+
.filter(errorObject -> errorObject instanceof ConcurrentModificationError)
66+
.findFirst()
67+
.map(errorObject -> ((ConcurrentModificationError) errorObject).getCurrentVersion()))
6568
.orElse(null);
6669
}
6770
}

‎commercetools/commercetools-sdk-java-api/src/test/java/com/commercetools/ErrorTest.java‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,19 +2,46 @@
22
package com.commercetools;
33

44
import static com.commercetools.TestUtils.stringFromResource;
5+
import static io.vrap.rmf.base.client.utils.ClientUtils.blockingWait;
56
import static org.assertj.core.api.Assertions.assertThat;
67

78
import java.io.IOException;
9+
import java.net.URI;
10+
import java.time.Duration;
11+
import java.util.concurrent.CompletableFuture;
812

13+
import com.commercetools.api.client.error.ApiHttpExceptionFactory;
914
import com.commercetools.api.models.error.ErrorObject;
1015
import com.commercetools.api.models.error.ErrorResponse;
1116
import com.commercetools.api.models.error.InvalidJsonInputError;
17+
import com.tngtech.junit.dataprovider.DataProvider;
18+
import com.tngtech.junit.dataprovider.DataProviderExtension;
19+
import com.tngtech.junit.dataprovider.UseDataProvider;
20+
import com.tngtech.junit.dataprovider.UseDataProviderExtension;
1221

22+
import io.vrap.rmf.base.client.*;
23+
import io.vrap.rmf.base.client.error.*;
24+
import io.vrap.rmf.base.client.http.ErrorMiddleware;
1325
import io.vrap.rmf.base.client.utils.json.JsonUtils;
1426

27+
import org.assertj.core.api.Assertions;
1528
import org.junit.jupiter.api.Test;
29+
import org.junit.jupiter.api.TestTemplate;
30+
import org.junit.jupiter.api.extension.ExtendWith;
1631

32+
@ExtendWith(DataProviderExtension.class)
33+
@ExtendWith(UseDataProviderExtension.class)
1734
public class ErrorTest {
35+
@DataProvider
36+
public static Object[][] exceptions() {
37+
return new Object[][] { { 400, BadRequestException.class }, { 401, UnauthorizedException.class },
38+
{ 403, ForbiddenException.class }, { 404, NotFoundException.class },
39+
{ 409, com.commercetools.api.client.error.ConcurrentModificationException.class },
40+
{ 499, ApiClientException.class }, { 500, InternalServerErrorException.class },
41+
{ 502, BadGatewayException.class }, { 503, ServiceUnavailableException.class },
42+
{ 504, GatewayTimeoutException.class }, { 599, ApiServerException.class }, };
43+
}
44+
1845
@Test
1946
public void deprecatedAttributesAccessor() throws IOException {
2047
ErrorResponse errorResponse = JsonUtils.fromJsonString(stringFromResource("errors.json"), ErrorResponse.class);
@@ -25,4 +52,19 @@ public void deprecatedAttributesAccessor() throws IOException {
2552
final InvalidJsonInputError error = (InvalidJsonInputError) errorObject;
2653
assertThat(error.getDetailedErrorMessage()).isEqualTo("actions -> name: Missing required value");
2754
}
55+
56+
@TestTemplate
57+
@UseDataProvider("exceptions")
58+
public void testErrorInvalidResponse(int statusCode, Class<ApiHttpException> exceptionClass) {
59+
ErrorMiddleware middleware = ErrorMiddleware.of(ApiHttpExceptionFactory.of(ResponseSerializer.of()));
60+
61+
final ApiHttpRequest request = new ApiHttpRequest(ApiHttpMethod.GET, URI.create("/"), new ApiHttpHeaders(),
62+
"".getBytes());
63+
final ApiHttpResponse<byte[]> response = new ApiHttpResponse<>(statusCode, null, null);
64+
65+
Assertions.assertThatExceptionOfType(exceptionClass).isThrownBy(() -> {
66+
blockingWait(middleware.invoke(request, request1 -> CompletableFuture.completedFuture(response)),
67+
Duration.ofSeconds(1));
68+
}).matches(e -> e.getStatusCode() == statusCode);
69+
}
2870
}

‎rmf/rmf-java-base/src/main/java/io/vrap/rmf/base/client/error/HttpExceptionFactory.java‎

Lines changed: 28 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11

22
package io.vrap.rmf.base.client.error;
33

4+
import java.util.Optional;
5+
46
import io.vrap.rmf.base.client.ApiHttpException;
57
import io.vrap.rmf.base.client.ApiHttpRequest;
68
import io.vrap.rmf.base.client.ApiHttpResponse;
@@ -25,51 +27,53 @@ public default ApiHttpException create(final ApiHttpRequest request, final ApiHt
2527
public default ApiHttpException createServerException(final ApiHttpRequest request,
2628
final ApiHttpResponse<byte[]> response) {
2729
final ResponseSerializer serializer = getResponseSerializer();
28-
String message = "Server error response [url] " + request.getUri().toString() + " [status code] "
30+
final String message = "Server error response [url] " + request.getUri().toString() + " [status code] "
2931
+ response.getStatusCode() + " [reason phrase] " + response.getMessage();
3032

33+
final String body = Optional.ofNullable(response.getBody()).map(String::new).orElse("");
3134
switch (response.getStatusCode()) {
3235
case HttpStatusCode.INTERNAL_SERVER_ERROR_500:
33-
return new InternalServerErrorException(response.getStatusCode(), new String(response.getBody()),
34-
request.getHeaders(), message, response, request, serializer);
36+
return new InternalServerErrorException(response.getStatusCode(), body, request.getHeaders(), message,
37+
response, request, serializer);
3538
case HttpStatusCode.BAD_GATEWAY_502:
36-
return new BadGatewayException(response.getStatusCode(), new String(response.getBody()),
37-
request.getHeaders(), message, response, request, serializer);
39+
return new BadGatewayException(response.getStatusCode(), body, request.getHeaders(), message, response,
40+
request, serializer);
3841
case HttpStatusCode.SERVICE_UNAVAILABLE_503:
39-
return new ServiceUnavailableException(response.getStatusCode(), new String(response.getBody()),
40-
request.getHeaders(), message, response, request, serializer);
42+
return new ServiceUnavailableException(response.getStatusCode(), body, request.getHeaders(), message,
43+
response, request, serializer);
4144
case HttpStatusCode.GATEWAY_TIMEOUT_504:
42-
return new GatewayTimeoutException(response.getStatusCode(), new String(response.getBody()),
43-
request.getHeaders(), message, response, request, serializer);
45+
return new GatewayTimeoutException(response.getStatusCode(), body, request.getHeaders(), message,
46+
response, request, serializer);
4447
}
45-
return new ApiServerException(response.getStatusCode(), new String(response.getBody()), response.getHeaders(),
46-
message, response, request);
48+
return new ApiServerException(response.getStatusCode(), body, response.getHeaders(), message, response,
49+
request);
4750
}
4851

4952
public default ApiHttpException createClientException(ApiHttpRequest request, ApiHttpResponse<byte[]> response) {
5053
final ResponseSerializer serializer = getResponseSerializer();
51-
String message = "Client error response [url] " + request.getUri().toString() + " [status code] "
54+
final String message = "Client error response [url] " + request.getUri().toString() + " [status code] "
5255
+ response.getStatusCode() + " [reason phrase] " + response.getMessage();
5356

57+
final String body = Optional.ofNullable(response.getBody()).map(String::new).orElse("");
5458
switch (response.getStatusCode()) {
5559
case HttpStatusCode.BAD_REQUEST_400:
56-
return new BadRequestException(response.getStatusCode(), new String(response.getBody()),
57-
request.getHeaders(), message, response, request, serializer);
60+
return new BadRequestException(response.getStatusCode(), body, request.getHeaders(), message, response,
61+
request, serializer);
5862
case HttpStatusCode.UNAUTHORIZED_401:
59-
return new UnauthorizedException(response.getStatusCode(), new String(response.getBody()),
60-
request.getHeaders(), message, response, request, serializer);
63+
return new UnauthorizedException(response.getStatusCode(), body, request.getHeaders(), message,
64+
response, request, serializer);
6165
case HttpStatusCode.FORBIDDEN_403:
62-
return new ForbiddenException(response.getStatusCode(), new String(response.getBody()),
63-
request.getHeaders(), message, response, request, serializer);
66+
return new ForbiddenException(response.getStatusCode(), body, request.getHeaders(), message, response,
67+
request, serializer);
6468
case HttpStatusCode.NOT_FOUND_404:
65-
return new NotFoundException(response.getStatusCode(), new String(response.getBody()),
66-
request.getHeaders(), message, response, request, serializer);
69+
return new NotFoundException(response.getStatusCode(), body, request.getHeaders(), message, response,
70+
request, serializer);
6771
case HttpStatusCode.CONFLICT_409:
68-
return new ConcurrentModificationException(response.getStatusCode(), new String(response.getBody()),
69-
request.getHeaders(), message, response, request, serializer);
72+
return new ConcurrentModificationException(response.getStatusCode(), body, request.getHeaders(),
73+
message, response, request, serializer);
7074
}
71-
return new ApiClientException(response.getStatusCode(), new String(response.getBody()), response.getHeaders(),
72-
message, response, request);
75+
return new ApiClientException(response.getStatusCode(), body, response.getHeaders(), message, response,
76+
request);
7377
}
7478

7579
public static HttpExceptionFactory of(final ResponseSerializer serializer) {

‎rmf/rmf-java-base/src/test/java/io/vrap/rmf/base/client/http/ErrorMiddlewareTest.java‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,4 +49,19 @@ public void testError(int statusCode, Class<ApiHttpException> exceptionClass) {
4949
Duration.ofSeconds(1));
5050
}).matches(e -> e.getStatusCode() == statusCode);
5151
}
52+
53+
@TestTemplate
54+
@UseDataProvider("exceptions")
55+
public void testErrorInvalidResponse(int statusCode, Class<ApiHttpException> exceptionClass) {
56+
ErrorMiddleware middleware = ErrorMiddleware.of(HttpExceptionFactory.of(ResponseSerializer.of()));
57+
58+
final ApiHttpRequest request = new ApiHttpRequest(ApiHttpMethod.GET, URI.create("/"), new ApiHttpHeaders(),
59+
"".getBytes());
60+
final ApiHttpResponse<byte[]> response = new ApiHttpResponse<>(statusCode, null, null);
61+
62+
Assertions.assertThatExceptionOfType(exceptionClass).isThrownBy(() -> {
63+
blockingWait(middleware.invoke(request, request1 -> CompletableFuture.completedFuture(response)),
64+
Duration.ofSeconds(1));
65+
}).matches(e -> e.getStatusCode() == statusCode);
66+
}
5267
}

0 commit comments

Comments
 (0)