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 #1970 - update MultiPartFormDataContent to allow contentType override using optional builder #2003

Merged
merged 3 commits into from Nov 23, 2021

Conversation

jschneidereit
Copy link
Contributor

Fixes #1970

Subsystem
Client

Motivation
Before this change MultiPartFormDataContent was not compatible with servers that accepted multipart/mixed content.

Solution
Now a user can pass their own lambda for building content type like so:

val mixed = MultiPartFormDataContent(
    parts = listOf(),
    buildContentType = { b -> ContentType.MultiPart.Mixed.withParameter("boundary", b) }
)

@e5l
Copy link
Member

e5l commented Jul 23, 2020

Hey @jschneidereit, sorry for the delay. Could you check one thing before the merge?

@jschneidereit
Copy link
Contributor Author

Hey @jschneidereit, sorry for the delay. Could you check one thing before the merge?

@e5l did I miss something?

@jschneidereit jschneidereit requested a review from e5l July 29, 2020 20:13
/**
* [OutgoingContent] for multipart/form-data formatted request.
*
* @param parts: form part data
*/
public class MultiPartFormDataContent(
parts: List<PartData>
parts: List<PartData>,
val boundary: String = generateBoundary(),
Copy link
Member

Choose a reason for hiding this comment

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

Could you tell me why do we need boundary in the constructor?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @e5l I had originally made it override-able but @cy6erGn0m requested an optional constructor argument in the discussion in #1970

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 other option to only having contentType in the constructor and not boundary would be a lambda that accepts a string like contentTypeFn: (String) -> ContentType but I think that's a little overkill.

@rsinukov rsinukov requested a review from e5l October 22, 2021 16:10
@e5l e5l merged commit e1270e9 into ktorio:main Nov 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Objections to changing boundary to internal on MultiPartFormDataContent?
2 participants