[2.2.0-RC2] Regression in routes handling

[benmccann]

This was legal in 2.1.4, but does not seem to work in the 2.2.0-RC2 release that I just grabbed from Maven.

POST  /someurl/:myparam  @controllers.MyController.execute(myparam)

When I submit the request, I get:

Bad request
'POST /someurl/5201785ee4b0f68890ef7599' [Invalid XML]

[huntc]

What's the payload you're passing out if interest

[benmccann]

There's no payload. The only param is in the URL

[huntc]

Are you using Chrome? Can you try sending via curl? Can you send the headers to us?

[jroper]

Does it happen with Firefox? I've seen a similar issue reported recently where it only happened for Chrome, not Firefox. What happened was that Chrome is submitting empty bodies with a Content-Type of application/xml. That's invalid, if the content type is application/xml, then you should be able to parse it with an XML parser, which is what Play is doing. But an empty String is invalid XML. I can't see anything in Play that has changed here, so I suspect it's actually a regression in Chrome that has coincided with Play 2.2, but I could be wrong.

[benmccann]

Yes, I'm using Chrome and it has set the Content-Type to "application/xml". There is something that has changed in Play though. If I send an empty body with "application/xml" in Play 2.1.4 that does work.

[jroper]

Ok, so the actual change here is that Play used to not recognise application/xml requests. If you send an empty text/xml post to Play 2.1.4, you get the same problem. But I can't reproduce the issue of setting the content type in Chrome, using vanilla XMLHttpRequest or jquery 1.9, when you don't parse a body, no content type is set.

[jroper]

Could you check to see if it happens in Firefox/Safari/IE? What version of AngularJS are you using?

[jroper]

I believe it's related to this bug:

https://code.google.com/p/chromium/issues/detail?id=172802

If you pass "" into XMLHttpRequest.send, you end up with a content type of application/xml. Which is both non spec compliant, and wrong. I suspect there's also a bug to be raised in AngularJS here, in that if you don't supply a body, it shouldn't pass a body to send, even an empty String.

[jroper]

And here's the line in AngularJS, I believe, that is sending empty Strings:

https://github.com/angular/angular.js/blob/003861d2fdb37b83e1d0939d49b70fbc67766997/src/ng/httpBackend.js#L106

[benmccann]

Ok, well I guess now that I know can just pass an empty JS object to Angular to get my app working that's good enough for me. I don't have a super strong feeling about whether Play or Chrome or Angular should change their behavior. Chrome's behavior seems the most wrong here.

[jroper]

So, I'm not 100% sure what to do here. I think what Play is doing is right. By design, Play's default body parsers parse bodies according to the content type. And Play has every right to parse a request body with Content-Type application/xml as XML. Should it ignore it if there is no body? I don't think so, what then should it do when the body is intentionally passed as application/xml, but unintentionally has no content? It should flag the error.

So, I don't think Play is the right place to fix this. The root cause of the issue is in angular (and any other JS framework that submits an empty string instead of null), and I've raised a PR that I hope fixes it (if you could test for me, that would be great):

angular/angular.js#4021

The Chrome bug exacerbates the issue, but isn't the root cause.

For now, I think the right thing to do is for us to publish a temporary work around that can be used until angular/chrome can be fixed. So here it is:

package filters

import play.api.mvc._
import play.api.http.HeaderNames._
import play.api.http.MimeTypes._

/**
 * Works around the issue where AngularJS on WebKit sends empty application/xml bodies
 */
class FixEmptyXmlFilter extends EssentialFilter {

  def apply(next: EssentialAction): EssentialAction = EssentialAction { req =>
    (req.headers.get(CONTENT_LENGTH), req.headers.get(CONTENT_TYPE)) match {
      case (None | Some("0"), Some(XML)) =>
        next(new WrappedRequest(Request(req, "")) {
            override lazy val mediaType = None
            override lazy val contentType = None
        })
      case _ => next(req)
    }
  }

}

Using this from a Java project:

import play.GlobalSettings;
import play.api.mvc.EssentialFilter;

public class Global extends GlobalSettings {
    public <T extends EssentialFilter> Class<T>[] filters() {
        return new Class[]{filters.FixEmptyXmlFilter.class};
    }
}

[benmccann]

thanks for looking into this one!

[huntc]

Sorry, chiming in late here. It is acceptable for a POST to contain no data. If there is no data it shouldn't be parsed no matter what the mime type... I think this could be a problem for us to deal with.

[jroper]

There's nothing in HTTP to differentiate no content and empty content. Both result in a 0 length request body. So there's no such thing as "a POST containing no data", there is only "a POST with an empty body". In the case, for example, of the text/plain;charset=utf-8 content type, the empty body is actually data, it's an empty text file. It may be parsed according to the encoding, and the result will be an empty String. The same goes for application/x-www-form-urlencoded, an empty body may be parsed (and we do parse it), and the result is an empty Map[String, Seq[String]]. It's not no data, it is a form with no fields, and must be parsed.

The important thing here is that the Content-Type is being used to say what type of body is there, and how it should be parsed. The presence of an application/xml content type indicates that the request body may and should be parsed as XML. Play therefore has every right to attempt to parse it, and can, and should, raise an error if the body is not in that format, which for an empty body, is the case. I think Play is exhibiting the correct behaviour here.

If a client is sending no data, ie, if the client is not sending content, then the client should not set a content type, since it is an error to describe the type of something that doesn't exist. Right?

[benmccann]

@jroper hmm, i don't know the details super well here. so if there's nothing in HTTP to differentiate no content and empty content, then how does your change to Angular to send null instead of empty string work? wouldn't those be interpreted as the same things? is it just a workaround for the Chrome bug?

[jroper]

There is something in the Javascript XMLHttpRequest API to differentiate between it (null or empty string), and based on that, a Content-Type header is or isn't set.

[ejain]

Thanks for the workaround, but it looks like this solution won't work in Eclipse without the Scala IDE?

[jroper]

This issue by the way is now both fixed in Chrome:

https://code.google.com/p/chromium/issues/detail?id=172802

And also in WebKit (hence eventually Safari):

https://bugs.webkit.org/show_bug.cgi?id=99973