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

Conversation

mickjermsurawong-stripe
Copy link
Contributor

  • This fixes encoding of nested params in multipart request. Currently, we assume that parameters used in multipart request is one-level deep and contains file object at root-level.
  • This PR allows nested params, and makes assumption of root-level file object explicit.
  • This is similar to the problem addressed in php Fix encoding of nested parameters in multipart requests stripe-php#624

r? @ob-stripe
cc @remi-stripe @stripe/api-libraries

+ 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

+ "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.

}

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.

Copy link
Contributor

@ob-stripe ob-stripe left a comment

Choose a reason for hiding this comment

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

Looks great! Thanks for the quick fix :)

+ "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

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.

@mickjermsurawong-stripe mickjermsurawong-stripe merged commit 1f8af54 into master Apr 11, 2019
@ob-stripe ob-stripe deleted the mickjermsurawong/fix-file-nested-params branch April 12, 2019 17:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants