-
Notifications
You must be signed in to change notification settings - Fork 451
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: use random UUID for multipart boundary delimiter #916
fix: use random UUID for multipart boundary delimiter #916
Conversation
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here with What to do if you already signed the CLAIndividual signers
Corporate signers
ℹ️ Googlers: Go here for more info. |
d4b8f7d
to
787b159
Compare
CLAs look good, thanks! ℹ️ Googlers: Go here for more info. |
787b159
to
9971579
Compare
|
||
public MultipartContent() { | ||
super(new HttpMediaType("multipart/related").setParameter("boundary", "__END_OF_PART__")); | ||
this("__END_OF_PART__" + java.util.UUID.randomUUID().toString() + "__"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason not to import java.util.UUID? e.g a conflict with a similarly named class?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed.
{"<xml>Hi</xml>", "application/xml"}, | ||
{"{x:1,y:2}", "application/json"} | ||
}; | ||
StringBuilder expectedContentSB = new StringBuilder(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no abbreviations per google style, but simply "expectedContent" or even just expected is fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
expectedContent is used later. renamed to expectedStringBuilder
ByteArrayOutputStream out = new ByteArrayOutputStream(); | ||
content.writeTo(out); | ||
String expectedContent = expectedContentSB.toString(); | ||
assertEquals(expectedContent, out.toString()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
out.toString needs to specify a locale
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed in two places. Thanks for catching that.
…en-plugin to v1.6.13 (googleapis#916) [![WhiteSource Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [org.sonatype.plugins:nexus-staging-maven-plugin](http://www.sonatype.com/) ([source](https://togithub.com/sonatype/nexus-maven-plugins)) | `1.6.12` -> `1.6.13` | [![age](https://badges.renovateapi.com/packages/maven/org.sonatype.plugins:nexus-staging-maven-plugin/1.6.13/age-slim)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://badges.renovateapi.com/packages/maven/org.sonatype.plugins:nexus-staging-maven-plugin/1.6.13/adoption-slim)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://badges.renovateapi.com/packages/maven/org.sonatype.plugins:nexus-staging-maven-plugin/1.6.13/compatibility-slim/1.6.12)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://badges.renovateapi.com/packages/maven/org.sonatype.plugins:nexus-staging-maven-plugin/1.6.13/confidence-slim/1.6.12)](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>sonatype/nexus-maven-plugins</summary> ### [`v1.6.13`](https://togithub.com/sonatype/nexus-maven-plugins/compare/release-1.6.12...release-1.6.13) [Compare Source](https://togithub.com/sonatype/nexus-maven-plugins/compare/release-1.6.12...release-1.6.13) </details> --- ### Configuration 📅 **Schedule**: At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, click this checkbox. --- This PR has been generated by [WhiteSource Renovate](https://renovate.whitesourcesoftware.com). View repository job log [here](https://app.renovatebot.com/dashboard#github/googleapis/google-auth-library-java).
Fixes #909
Fixes googleapis/google-cloud-java#6386