Fix handling multiple request form - #2048
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces support for binary and multipart/form-data request bodies, adding new classes like BinaryRequestBody, HttpFile, and RequestForm, and updating AdyenHttpClient and API templates to handle file uploads and form parameters. The review feedback highlights several opportunities to improve robustness by adding defensive null checks in constructors and methods across the new classes to prevent potential NullPointerException issues.
| public BinaryRequestBody(byte[] data, String contentType) { | ||
| this.data = Arrays.copyOf(data, data.length); | ||
| this.contentType = contentType; | ||
| } |
There was a problem hiding this comment.
To prevent a potential NullPointerException when data is null, we should add a defensive null check in the constructor. If data is null, we can default to an empty byte array.
| public BinaryRequestBody(byte[] data, String contentType) { | |
| this.data = Arrays.copyOf(data, data.length); | |
| this.contentType = contentType; | |
| } | |
| public BinaryRequestBody(byte[] data, String contentType) { | |
| this.data = data != null ? Arrays.copyOf(data, data.length) : new byte[0]; | |
| this.contentType = contentType; | |
| } |
| public HttpFile(byte[] data, String name) { | ||
| this.data = Arrays.copyOf(data, data.length); | ||
| this.name = name; | ||
| } |
There was a problem hiding this comment.
To prevent a potential NullPointerException when data is null, we should add a defensive null check in the constructor. If data is null, we can default to an empty byte array.
| public HttpFile(byte[] data, String name) { | |
| this.data = Arrays.copyOf(data, data.length); | |
| this.name = name; | |
| } | |
| public HttpFile(byte[] data, String name) { | |
| this.data = data != null ? Arrays.copyOf(data, data.length) : new byte[0]; | |
| this.name = name; | |
| } |
| public void add(String name, HttpFile file) { | ||
| if (!multipart) { | ||
| throw new IllegalStateException("Files require a multipart/form-data request"); | ||
| } | ||
| multipartBuilder.addBinaryBody( | ||
| name, file.getData(), ContentType.DEFAULT_BINARY, file.getName()); | ||
| } |
There was a problem hiding this comment.
To prevent a potential NullPointerException when file is null, we should add a defensive null check and throw a clear IllegalArgumentException.
public void add(String name, HttpFile file) {
if (!multipart) {
throw new IllegalStateException("Files require a multipart/form-data request");
}
if (file == null) {
throw new IllegalArgumentException("File cannot be null");
}
multipartBuilder.addBinaryBody(
name, file.getData(), ContentType.DEFAULT_BINARY, file.getName());
}| private static String normalizeMediaType(String contentType) { | ||
| int parameterIndex = contentType.indexOf(';'); | ||
| return contentType | ||
| .substring(0, parameterIndex < 0 ? contentType.length() : parameterIndex) | ||
| .trim() | ||
| .toLowerCase(); | ||
| } |
There was a problem hiding this comment.
To prevent a potential NullPointerException when contentType is null, we should add a defensive null check at the beginning of normalizeMediaType.
private static String normalizeMediaType(String contentType) {
if (contentType == null) {
return "";
}
int parameterIndex = contentType.indexOf(';');
return contentType
.substring(0, parameterIndex < 0 ? contentType.length() : parameterIndex)
.trim()
.toLowerCase();
}| HttpUriRequestBase createRequest( | ||
| String endpoint, | ||
| BinaryRequestBody requestBody, | ||
| Config config, | ||
| boolean isApiKeyRequired, | ||
| RequestOptions requestOptions, | ||
| ApiConstants.HttpMethod httpMethod, | ||
| Map<String, String> params) | ||
| throws HTTPClientException { | ||
| HttpUriRequestBase httpRequest = | ||
| createHttpRequestBase(createUri(endpoint, params), requestBody.getData(), httpMethod); |
There was a problem hiding this comment.
To prevent a potential NullPointerException when requestBody is null, we should add a defensive null check at the beginning of createRequest.
| HttpUriRequestBase createRequest( | |
| String endpoint, | |
| BinaryRequestBody requestBody, | |
| Config config, | |
| boolean isApiKeyRequired, | |
| RequestOptions requestOptions, | |
| ApiConstants.HttpMethod httpMethod, | |
| Map<String, String> params) | |
| throws HTTPClientException { | |
| HttpUriRequestBase httpRequest = | |
| createHttpRequestBase(createUri(endpoint, params), requestBody.getData(), httpMethod); | |
| HttpUriRequestBase createRequest( | |
| String endpoint, | |
| BinaryRequestBody requestBody, | |
| Config config, | |
| boolean isApiKeyRequired, | |
| RequestOptions requestOptions, | |
| ApiConstants.HttpMethod httpMethod, | |
| Map<String, String> params) | |
| throws HTTPClientException { | |
| if (requestBody == null) { | |
| throw new IllegalArgumentException("requestBody cannot be null"); | |
| } | |
| HttpUriRequestBase httpRequest = | |
| createHttpRequestBase(createUri(endpoint, params), requestBody.getData(), httpMethod); |
|



Description
Tested scenarios
Fixed issue: