Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix encoding of nested parameters in multipart requests #735

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
99 changes: 59 additions & 40 deletions src/main/java/com/stripe/net/LiveStripeResponseGetter.java
Original file line number Diff line number Diff line change
Expand Up @@ -367,6 +367,13 @@ private static List<Parameter> flattenParamsValue(Object value, String keyPrefix
} else if (value == null) {
flatParams = new ArrayList<>();
flatParams.add(new Parameter(keyPrefix, ""));
} else if ((value instanceof File) || (value instanceof InputStream)) {
throw new InvalidRequestException(String.format(
"java.io.File or java.io.InputStream %s is not supported at '%s' parameter. "
+ "Please check our API reference for the parameter type, "
+ "or use the provided parameter class instead.",
value, keyPrefix),
keyPrefix, null, null, 0, null);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error here could arise if:

  • users pass file object to non-multipart request at any level.
  • multi-part request params having nested file objects

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Excellent, thanks for adding an explicit check and error.

} else {
flatParams = new ArrayList<>();
flatParams.add(new Parameter(keyPrefix, value.toString()));
Expand Down Expand Up @@ -607,46 +614,8 @@ private static StripeResponse getMultipartStripeResponse(

MultipartProcessor multipartProcessor = null;
try {
multipartProcessor = new MultipartProcessor(
conn, boundary, ApiResource.CHARSET);

for (Map.Entry<String, Object> entry : params.entrySet()) {
String key = entry.getKey();
Object value = entry.getValue();

if (value instanceof File) {
File currentFile = (File) value;
if (!currentFile.exists()) {
throw new InvalidRequestException("File for key "
+ key + " must exist.", null, null, null, 0, null);
} else if (!currentFile.isFile()) {
throw new InvalidRequestException("File for key "
+ key
+ " must be a file and not a directory.",
null, null, null, 0, null);
} else if (!currentFile.canRead()) {
throw new InvalidRequestException(
"Must have read permissions on file for key "
+ key + ".", null, null, null, 0, null);
}
multipartProcessor.addFileField(key, currentFile.getName(),
new FileInputStream(currentFile));
} else if (value instanceof InputStream) {
@Cleanup InputStream inputStream = (InputStream) value;
if (inputStream.available() == 0) {
throw new InvalidRequestException(
"Must have available bytes to read on InputStream for key "
+ key + ".", null, null, null, 0, null
);
}
multipartProcessor.addFileField(key, "blob", inputStream);
} else {
// We only allow a single level of nesting for params
// for multipart
multipartProcessor.addFormField(key, (String) value);
}
}

multipartProcessor = new MultipartProcessor(conn, boundary, ApiResource.CHARSET);
encodeMultipartParams(multipartProcessor, params);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternative design here is to have encodeParams as a method to multipartprocessor itself.
However, we still want to use the same flattenParams and LiveRequestGetter.Parameter within in class.

} finally {
if (multipartProcessor != null) {
multipartProcessor.finish();
Expand Down Expand Up @@ -682,6 +651,56 @@ private static StripeResponse getMultipartStripeResponse(

}

/**
* Encode multipart params as a counter-part method to {@link this#createQuery(Map)} for
* encoding params for non-multipart request.
* @param multipartProcessor multi-part processor handling encoding of input stream and
* basic key-value forms.
* @param params parameter map that can contain file or input stream.
*/
static void encodeMultipartParams(MultipartProcessor multipartProcessor,
Map<String, Object> params)
throws InvalidRequestException, IOException {

for (Map.Entry<String, Object> entry : params.entrySet()) {
String key = entry.getKey();
Object value = entry.getValue();

if (value instanceof File) {
File currentFile = (File) value;
if (!currentFile.exists()) {
throw new InvalidRequestException("File for key "
+ key + " must exist.", null, null, null, 0, null);
} else if (!currentFile.isFile()) {
throw new InvalidRequestException("File for key "
+ key
+ " must be a file and not a directory.",
null, null, null, 0, null);
} else if (!currentFile.canRead()) {
throw new InvalidRequestException(
"Must have read permissions on file for key "
+ key + ".", null, null, null, 0, null);
}
multipartProcessor.addFileField(key, currentFile.getName(),
new FileInputStream(currentFile));
} else if (value instanceof InputStream) {
@Cleanup InputStream inputStream = (InputStream) value;
if (inputStream.available() == 0) {
throw new InvalidRequestException(
"Must have available bytes to read on InputStream for key "
+ key + ".", null, null, null, 0, null
);
}
multipartProcessor.addFileField(key, "blob", inputStream);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The two branches above are the same as before.
Only else branch below delegates the the flatten params used by createQuery in normal request method type

} else {
List<Parameter> parameters = flattenParamsValue(value, key);
for (Parameter parameter : parameters) {
multipartProcessor.addFormField(parameter.key, parameter.value);
}
}
}
}

private static void raiseMalformedJsonError(String responseBody, int responseCode,
String requestId) throws ApiException {
throw new ApiException(
Expand Down
10 changes: 9 additions & 1 deletion src/test/java/com/stripe/functional/FileTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,10 @@ public void testCreateWithFileWithTypedParams() throws StripeException {
File fileObject = new File(getClass().getResource("/test.png").getFile());
FileCreateParams fileCreateParams = FileCreateParams.builder()
.setPurpose(FileCreateParams.Purpose.DISPUTE_EVIDENCE)
.setFileLinkData(FileCreateParams.FileLinkData.builder()
.setCreate(true)
.setExpiresAt(123L)
.build())
.setFile(fileObject)
.build();

Expand All @@ -58,7 +62,11 @@ public void testCreateWithFileWithTypedParams() throws StripeException {
"/v1/files",
ImmutableMap.of(
"purpose", "dispute_evidence",
"file", fileObject
"file", fileObject,
"file_link_data", ImmutableMap.of(
"create", true,
"expires_at", 123
)
),
ApiResource.RequestType.MULTIPART,
null
Expand Down
49 changes: 49 additions & 0 deletions src/test/java/com/stripe/net/LiveStripeResponseGetterTest.java
Original file line number Diff line number Diff line change
@@ -1,22 +1,32 @@
package com.stripe.net;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

import com.google.gson.Gson;
import com.google.gson.reflect.TypeToken;

import com.stripe.Stripe;
import com.stripe.exception.InvalidRequestException;
import com.stripe.exception.StripeException;
import com.stripe.net.RequestOptions.RequestOptionsBuilder;

import java.io.ByteArrayOutputStream;
import java.io.File;
import java.io.IOException;
import java.io.UnsupportedEncodingException;
import java.net.HttpURLConnection;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;

import org.hamcrest.CoreMatchers;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

Expand Down Expand Up @@ -164,6 +174,45 @@ public void testCorrectAdditionalOwners() throws StripeException, UnsupportedEnc
LiveStripeResponseGetter.createQuery(params));
}

@Test
public void testThrowingOnCreateQueryWithFile() {
final Map<String, Object> params = new HashMap<>();
params.put("file", new File(getClass().getResource("/test.png").getFile()));

// file encoding should be handled in multi-part request instead
assertThrows(InvalidRequestException.class, () -> {
LiveStripeResponseGetter.createQuery(params);
});
}

@Test
public void testEncodeMultipartParams() throws IOException, InvalidRequestException {
final HttpURLConnection mockConn = mock(HttpURLConnection.class);
final ByteArrayOutputStream outputStream = new ByteArrayOutputStream();
when(mockConn.getOutputStream()).thenReturn(outputStream);
final MultipartProcessor multipartProcessor = new MultipartProcessor(
mockConn, "abc", ApiResource.CHARSET);

final Map<String, Object> params = new HashMap<>();
params.put("file", new File(getClass().getResource("/test.png").getFile()));

final Map<String, Object> nestedParams = new HashMap<>();
nestedParams.put("create", true);
nestedParams.put("expires_at", 123L);
params.put("file_link_data", nestedParams);

LiveStripeResponseGetter.encodeMultipartParams(multipartProcessor, params);

String encoded = outputStream.toString();

assertThat(encoded, CoreMatchers.containsString(
"Content-Disposition: form-data; name=\"file\"; filename=\"test.png\""));
assertThat(encoded, CoreMatchers.containsString(
"Content-Disposition: form-data; name=\"file_link_data[create]\""));
assertThat(encoded, CoreMatchers.containsString(
"Content-Disposition: form-data; name=\"file_link_data[expires_at]\""));
}

@Test
public void testAppInfo() {
final RequestOptions options = new RequestOptionsBuilder().setApiKey("sk_foobar").build();
Expand Down