[go: nahoru, domu]

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

MultipartContent class uses the same simple string as the default boundary #909

Closed
fazunenko opened this issue Dec 10, 2019 · 3 comments · Fixed by #916
Closed

MultipartContent class uses the same simple string as the default boundary #909

fazunenko opened this issue Dec 10, 2019 · 3 comments · Fixed by #916
Assignees
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@fazunenko
Copy link

com.google.api.client.http.MultipartContent class by default uses constant '__END_OF_PART__' string as the boundary (part delimiter).
It will cause problem if the data to be transfered includes this string. Such problems could be really hard to investigate.
Example of such problem:
googleapis/google-cloud-java#6386

The class also provides setBoundary(String) method to alter the default boundary, but this method is not accessible from the other project.

@fazunenko
Copy link
Author

Suggested fix: use a random UUID string as boundary. It will be next to impossible get bump into such value.

public MultipartContent() {
   super(new HttpMediaType("multipart/related").setParameter("boundary", "__END_OF_PART__"));
}

=>

public MultipartContent() {
    this("____" + java.util.UUID.randomUUID().toString() + "___");
 }
 
public MultipartContent(String boundary) {
    super(new HttpMediaType("multipart/related").setParameter("boundary", boundary));
}

@chanseokoh
Copy link
Contributor

Wouldn't it be more efficient to fix and use a constant (static final String, some random UUID) instead of creating a new UUID everytime the instance is created? (I'm genuinely asking.)

@fazunenko
Copy link
Author

I think, generating UUID each time when an instance of MultipartContent is created is a very little overhead.
Having any static string value publicly available will significantly increase the likelihood of being included into someone content. (At least it's possible). With a random value it's very unlikely to repeat that value.

@yoshi-automation yoshi-automation added the triage me I really want to be triaged. label Dec 11, 2019
@chingor13 chingor13 added priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. labels Dec 11, 2019
@yoshi-automation yoshi-automation removed the triage me I really want to be triaged. label Dec 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants