[2.4.0-M3] Bug: CSRF cookie not set when using application.context

[jroper]

To reproduce:

• Create a plain new play-java project.
• Add the Global CSRF filter to Global.java like shown in the docs
• Add filters to the libraryDependencies in build.sbt
• In your application.conf set

    application.context=/abc
    csrf.cookie.name=csrfcookie

• Make sure you use Milestone 3 in your project/plugins.sbt

    addSbtPlugin("com.typesafe.play" % "sbt-plugin" % "2.4.0-M3")

• Open your browser and make sure you clean your cookies
• Run the project and goto http://127.0.0.1:9000/abc.

Result: The csrfcookie does not get set (which also causes "Missing CSRF Token" exceptions when e.g. using @CSRF.formField in a view).
When using the exact same project and switching to 2.3.8 it works.
(Make sure you always clean the cookies between browser refreshes.)

[jroper]

Does it happen if you go to a sub path of the application context? Eg, /abc/def?

[mkurz]

@jroper Just tested it: No, it does not happen on a sub path.

[mkurz]

@jroper Also testet it with latest SNAPSHOT (7c4a4b6). Same here, does not work on /abc but works on /abc/def

[jroper]

Thanks for verifying that, should hopefully be straight forward to track down and fix.

[jroper]

PR attached. Also found a bug in our error handling where filters weren't applied to the not found handler.

[richdougherty]

Since I was just working on optimizing this (#4164), I have a suggestion for a faster check for the context. It should be faster than the regex used in DefaultHttpErrorHandler and faster than the double string traversal used in GlobalSettings.

def inContext(context: String, path: String): Boolean = {
  // Assume context is a string without a trailing '/'.
  // Handle three cases:
  // * !path.startsWith(context)
  //   - Either path is shorter than context or starts with a different prefix.
  // * path.startsWith(context) && path.length == context.length
  //   - Path is equal to context.
  // * path.startsWith(context) && path.charAt(context.length) == '/')
  //   - Path starts with context followed by a '/' character.
  path.startsWith(context) && (path.length == context.length || path.charAt(context.length) == '/')
}

[jroper]

@richdougherty I used your logic, plus an additional check - if the context is empty (which is by far the most common Play configuration, not many people use application contexts), then there's no context so everything is in context. So I used context.isEmpty to short circuit all the other checks. In GlobalSettings, I just added the short circuit, and not the other checks, so it will be fast in the default, most common case, and slow in other cases, but that doesn't matter because users can switch to the DefaultHttpRequestHandler, and we're going to delete GlobalSettings soon anyway.

[richdougherty]

Build error:

[error] /scratch/jenkins/workspace/play-master-PRs/framework/src/play/src/main/scala/play/api/http/HttpRequestHandler.scala:147: type mismatch;
[error]  found   : Unit
[error]  required: Boolean
[error]         case action: EssentialAction if inContext(request.path) => filterAction(action)
[error]                                                  ^

[jroper]

Maybe I should try compiling before pushing? Nah... that's what CI is for.

[richdougherty]

;)