Skip to content

MimeType: Multipart Boundary value should allow = #26698

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

Closed
bekker opened this issue Mar 18, 2021 · 7 comments
Closed

MimeType: Multipart Boundary value should allow = #26698

bekker opened this issue Mar 18, 2021 · 7 comments
Assignees
Labels
status: feedback-provided Feedback has been provided status: invalid An issue that we don't feel is valid

Comments

@bekker
Copy link

bekker commented Mar 18, 2021

Some HTTP clients use boundary value including = when sending multipart requests, like below:

Content-Type: multipart/form-data; boundary====1616056691487===

It seems = is fine for boundary value per RFC 2046 5.1.1:

   The only mandatory global parameter for the "multipart" media type is
   the boundary parameter, which consists of 1 to 70 characters from a
   set of characters known to be very robust through mail gateways, and
   NOT ending with white space. (If a boundary delimiter line appears to
   end with white space, the white space must be presumed to have been
   added by a gateway, and must be deleted.)  It is formally specified
   by the following BNF:

     boundary := 0*69<bchars> bcharsnospace

     bchars := bcharsnospace / " "

     bcharsnospace := DIGIT / ALPHA / "'" / "(" / ")" /
                      "+" / "_" / "," / "-" / "." /
                      "/" / ":" / "=" / "?"

But org.springframework.util.MimeType doesn't allow = characters, because it is considered as separator.

Spring should allow = characters for compatibility with such HTTP clients.

Edit: It used to work with Spring 4.X (Spring Boot 1.X)

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Mar 18, 2021
@poutsma poutsma self-assigned this Mar 23, 2021
@poutsma
Copy link
Contributor

poutsma commented Mar 23, 2021

I just checked, and parsing multipart/form-data; boundary====1616056691487=== in 4.3.x throws an InvalidMimeTypeException just as 5.3.x does. Which specific version 4.x. are you using?

The reason for not allowing the = is that HTTP 1.1 does not allow "separators" such as = as parameter values. See RFC 2616, section 3.6:

Parameters are in  the form of attribute/value pairs.

       parameter               = attribute "=" value
       attribute               = token
       value                   = token | quoted-string
       token          = 1*<any CHAR except CTLs or separators>
       separators     = "(" | ")" | "<" | ">" | "@"
                      | "," | ";" | ":" | "\" | <">
                      | "/" | "[" | "]" | "?" | "="
                      | "{" | "}" | SP | HT

So it seems we are dealing with contradictory RFCs here, which is always interesting.

What kind of client is sending these boundaries? If it's possible to change the code in order to quote the boundary (multipart/form-data; boundary="===1616056691487==="), then it should work.

@poutsma poutsma added the status: waiting-for-feedback We need additional information before we can continue label Mar 23, 2021
@bekker
Copy link
Author

bekker commented Mar 23, 2021

I use spring 4.3.8 (spring boot 1.5.3).

I checked myself if MimeType was the real problem, and you are right, 4.3.x also throws InvalidMimeTypeException.

The problem was that I switched to the Spring Reactive.

StandardServletMultipartResolver resolves such malformed boundary requests just fine, even if MimeType throws an exception (because it doesn't check one, and the actual parsing is done by org.apache.catalina.connector.Request)

But DefaultServerWebExchange which is default one for spring reactive, fails to parse such requests.

org.springframework.web.server.adapter.DefaultServerWebExchange
        private static Mono<MultiValueMap<String, Part>> initMultipartData(ServerHttpRequest request,
			ServerCodecConfigurer configurer, String logPrefix) {

		try {
			MediaType contentType = request.getHeaders().getContentType();
			if (MediaType.MULTIPART_FORM_DATA.isCompatibleWith(contentType)) {
				return ((HttpMessageReader<MultiValueMap<String, Part>>) configurer.getReaders().stream()
						.filter(reader -> reader.canRead(MULTIPART_DATA_TYPE, MediaType.MULTIPART_FORM_DATA))
						.findFirst()
						.orElseThrow(() -> new IllegalStateException("No multipart HttpMessageReader.")))
						.readMono(MULTIPART_DATA_TYPE, request, Hints.from(Hints.LOG_PREFIX_HINT, logPrefix))
						.switchIfEmpty(EMPTY_MULTIPART_DATA)
						.cache();
			}
		}
		catch (InvalidMediaTypeException ex) {
			// Ignore
		}
		return EMPTY_MULTIPART_DATA;
	}

The line MediaType contentType = request.getHeaders().getContentType(); throws InvalidMimeTypeException so it ignores the multipart data.

I see, those RFCs are contradictory :(
The client is from a somewhat old Android app so I can't identify the exact HTTP client, nor fix the app itself.
I'll try to reach the app developer and see if the app can be updated.

Thank you for checking.

EDIT:
I studied RFC 2046 5.1.1 more and there was a warning:

   WARNING TO IMPLEMENTORS:  The grammar for parameters on the Content-
   type field is such that it is often necessary to enclose the boundary
   parameter values in quotes on the Content-type line.  This is not
   always necessary, but never hurts. Implementors should be sure to
   study the grammar carefully in order to avoid producing invalid
   Content-type fields.  Thus, a typical "multipart" Content-Type header
   field might look like this:

     Content-Type: multipart/mixed; boundary=gc0p4Jq0M2Yt08j34c0p

   But the following is not valid:

     Content-Type: multipart/mixed; boundary=gc0pJq0M:08jU534c0p

   (because of the colon) and must instead be represented as

     Content-Type: multipart/mixed; boundary="gc0pJq0M:08jU534c0p"

So the header multipart/form-data; boundary====1616056691487=== was a malformed one, after all :)

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Mar 23, 2021
@abialas
Copy link

abialas commented Mar 23, 2021

I have a similar problem but with MediaType and MultipartParser. My header value let's say is:

multipart/form-data; boundary="===1616056691487==="

I have it set as Content-Type for multipart message and then I try to parse this message.
However, in the constructor of org.springframework.http.codec.multipart.MultipartParser.PreambleState the firstBoundary is set to:

this.firstBoundary = DataBufferUtils.matcher(MultipartUtils.concat(TWO_HYPHENS, MultipartParser.this.boundary));

which is obviously wrong because MultipartParser.this.boundary is quoted and afterwards this frst boundary could not be found. How can it be fixed? Is it a known issue?

@poutsma
Copy link
Contributor

poutsma commented Mar 24, 2021

So the header multipart/form-data; boundary====1616056691487=== was a malformed one, after all :)

@bekker Ok, good. Unless you think otherwise, I will close this issue.

@abialas Are you running the latest version? If not, I believe you ran into #26616, which was fixed in 5.3.5. If you are already running 5.3.5, please report a new issue.

@pietrucha
Copy link

pietrucha commented Mar 24, 2021

I also have problem with MimeType. In my case I am using Spring Boot v2.3.7 (spring-core:5.2.12.RELEASE) as proxy to request Geoserver services.
Geoserver in few cases return content as text/xml subtype=gml/3.1.1 Spring return me: Invalid token character '/' in token "text/xml subtype=gml/3.1.1".

Trying to create new MimeType("text/xml") also get error Invalid token character '/' in token "text/xml".
@poutsma Is there is workaround?

Edit:
new MimeType("text/xml")- throw exception, but MimeTypeUtils.parseMimeType("text/xml") - is OK.
Edit 2:
I want to handle incorrect content type response in Webflux -> WebClient.

@poutsma
Copy link
Contributor

poutsma commented Mar 25, 2021

@pietrucha wrote

Geoserver in few cases return content as text/xml subtype=gml/3.1.1

That does not look like a valid media type to me. The closest media type that conforms to the specification—which is also accepted by MimeTypeUtils::parseMimeType—would be something like text/xml;subtype="gml/3.1.1", where

  • I made the subtype a parameter by inserting a ;
  • I quoted the subtype parameter value, because it contains an invalid parameter (/), see also my comment here.

@pietrucha
Copy link

@poutsma thanks. I also found this in rfc2045#secition-5.1.

@poutsma poutsma added status: invalid An issue that we don't feel is valid and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Mar 31, 2021
@poutsma poutsma closed this as completed Mar 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: feedback-provided Feedback has been provided status: invalid An issue that we don't feel is valid
Projects
None yet
Development

No branches or pull requests

5 participants