-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
KTOR-7596 Make multipart Content-Type check case-insensitive #4413
Conversation
b9e5dd1
to
8b32a76
Compare
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.
lgtm
@@ -72,7 +71,7 @@ internal class JettyKtorHandler( | |||
) { | |||
try { | |||
val contentType = request.contentType | |||
if (contentType != null && contentType.startsWith("multipart/")) { | |||
if (contentType != null && contentType.startsWith("multipart/", ignoreCase = true)) { |
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.
I would extract constant here
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.
I could add a constant ContentType.Application.TYPE
. I also thought about adding an operator function contains
:
public object ContentType {
public object Application {
public const val TYPE: String = "application"
// ...
public operator fun contains(contentType: CharSequence): Boolean =
contentType.startsWith("$TYPE/", ignoreCase = true)
}
}
And then check the content type using in
:
if (contentType in ContentType.Application) { ... }
What do you think?
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.
I'll make it in a separate PR (if needed)
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.
@bjhham, what do you think?
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.
I've already done some experiments locally, so here you can find a draft PR: #4433
Let's discuss there
Subsystem
Server,
ktor-http-cio
Motivation
KTOR-7596 receiveMultipart fails with "IOException: Failed to parse multipart" when content-type is capitalized
Solution
Use case-insensitive check for multipart content-type