Merge branch 'master' of github.com:scalatra/scalatra

CREDITS.md

@@ -7,6 +7,7 @@
 - [Ross A. Baker](http://www.rossabaker.com/)
 - [Hiram Chirino](http://hiramchirino.com/)
 - [Ivan Porto Carrero](http://flanders.co.nz/) 
+- [Jared Armstrong](http://www.jaredarmstrong.name/) 
 
 ## Other contributors
 

fileupload/src/main/scala/org/scalatra/fileupload/FileUploadSupport.scala

@@ -3,7 +3,7 @@ package fileupload
 
 import org.apache.commons.fileupload.servlet.ServletFileUpload
 import org.apache.commons.fileupload.{FileItemFactory, FileItem}
-import org.apache.commons.fileupload.disk.DiskFileItemFactory
+import org.apache.commons.fileupload.disk.{DiskFileItem, DiskFileItemFactory}
 import collection.JavaConversions._
 import scala.util.DynamicVariable
 import java.util.{List => JList, HashMap => JHashMap, Map => JMap}
@@ -49,14 +49,36 @@ trait FileUploadSupport extends ScalatraKernel {
         val items = upload.parseRequest(req).asInstanceOf[JList[FileItem]]
         val bodyParams = items.foldRight(BodyParams(Map.empty, Map.empty)) { (item, params) =>
           if (item.isFormField)
-            BodyParams(params.fileParams, params.formParams + ((item.getFieldName, item.getString :: params.formParams.getOrElse(item.getFieldName, List[String]()))))
+            BodyParams(params.fileParams, params.formParams + ((item.getFieldName, fileItemToString(req, item) :: params.formParams.getOrElse(item.getFieldName, List[String]()))))
           else
             BodyParams(params.fileParams + ((item.getFieldName, item :: params.fileParams.getOrElse(item.getFieldName, List[FileItem]()))), params.formParams)
           }
         req(BodyParamsKey) = bodyParams
         bodyParams
     }
 
+  /**
+   * Converts a file item to a string.
+   *
+   * Browsers tend to be sloppy about providing content type headers with
+   * charset information to form-data parts.  Worse, browsers are
+   * inconsistent about how they encode these parameters. 
+   *
+   * The default implementation attempts to use the charset specified on
+   * the request.  If that is unspecified, and it usually isn't, then it
+   * falls back to the kernel's charset.
+   */
+  protected def fileItemToString(req: HttpServletRequest, item: FileItem): String = {
+    val charset = item match {
+      case diskItem: DiskFileItem =>
+        // Why doesn't FileItem have this method???
+        Option(diskItem.getCharSet())
+      case _ => 
+        None
+    }
+    item.getString(charset getOrElse defaultCharacterEncoding)
+  }
+
   private def wrapRequest(req: HttpServletRequest, formMap: Map[String, Seq[String]]) =
     new HttpServletRequestWrapper(req) {
       override def getParameter(name: String) = formMap.get(name) map { _.head } getOrElse null

fileupload/src/test/resources/org/scalatra/fileupload/multipart_request.txt

@@ -1,32 +1,36 @@
 POST ${PATH} HTTP/1.0
-Content-Type: multipart/form-data; boundary=---------------------------768277191987740441310253794
-Content-Length: 816
+Content-Type: multipart/form-data; boundary=---------------------------3924013385056820061124200860
+Content-Length: 944
 
------------------------------768277191987740441310253794
+-----------------------------3924013385056820061124200860
 Content-Disposition: form-data; name="string"
 
 foo
------------------------------768277191987740441310253794
+-----------------------------3924013385056820061124200860
+Content-Disposition: form-data; name="utf8-string"
+
+föo
+-----------------------------3924013385056820061124200860
 Content-Disposition: form-data; name="file"; filename="1.txt"
 Content-Type: text/plain
 
 one
 
------------------------------768277191987740441310253794
+-----------------------------3924013385056820061124200860
 Content-Disposition: form-data; name="file-none"; filename=""
 Content-Type: application/octet-stream
 
 
------------------------------768277191987740441310253794
+-----------------------------3924013385056820061124200860
 Content-Disposition: form-data; name="file-multi"; filename="2.txt"
 Content-Type: text/plain
 
 two
 
------------------------------768277191987740441310253794
+-----------------------------3924013385056820061124200860
 Content-Disposition: form-data; name="file-multi"; filename="3.txt"
 Content-Type: text/plain
 
 three
 
------------------------------768277191987740441310253794--
+-----------------------------3924013385056820061124200860--

fileupload/src/test/scala/org/scalatra/fileupload/FileUploadSupportTest.scala

@@ -1,6 +1,7 @@
 package org.scalatra
 package fileupload
 
+import java.net.{URLDecoder, URLEncoder}
 import org.scalatest.FunSuite
 import org.eclipse.jetty.testing.{ServletTester, HttpTester}
 import org.apache.commons.io.IOUtils
@@ -18,10 +19,10 @@ class FileUploadSupportTestServlet extends ScalatraServlet with FileUploadSuppor
       response.setHeader("file-multi-all", fis.foldLeft(""){ (acc, fi) => acc + new String(fi.get) })
     }
     params.get("file") foreach { response.setHeader("file-as-param", _) }
+    params("utf8-string")
   }
 
   post("/multipart-pass") {
-    println("PASSING")
     pass()
   }
 
@@ -42,8 +43,11 @@ class FileUploadSupportTest extends ScalatraFunSuite {
   addServlet(classOf[FileUploadSupportTestServlet], "/")
 
   def multipartResponse(path: String = "/multipart") = {
-    val req = IOUtils.toString(getClass.getResourceAsStream("multipart_request.txt"))
-      .replace("${PATH}", path)
+    // TODO We've had problems with the tester not running as iso-8859-1, even if the
+    // request really isn't iso-8859-1.  This is a hack, but this hack passes iff the
+    // browser behavior is correct.
+    val req = new String(IOUtils.toString(getClass.getResourceAsStream("multipart_request.txt"))
+      .replace("${PATH}", path).getBytes, "iso-8859-1")
     val res = new HttpTester("iso-8859-1")
     res.parse(tester.getResponses(req))
     res
@@ -53,6 +57,10 @@ class FileUploadSupportTest extends ScalatraFunSuite {
     multipartResponse().getHeader("string") should equal ("foo")
   }
 
+  test("decodes input parameters according to request encoding") {
+    multipartResponse().getContent() should equal ("föo")
+  }
+
   test("sets file params") {
     multipartResponse().getHeader("file") should equal ("one")
   }

notes/2.0.0.M5.markdown

@@ -9,6 +9,7 @@
 
 ## scalatra-fileupload
 * Keep query parameters available on multipart requests. [(GH-80)](http://github.com/scalatra/scalatra/issues/80)
+* Improve character decoding of form fields in multipart requests. [(GH-83)](http://github.com/scalatra/scalatra/issues/83)
 
 ## scalatra-socketio
 * Change interface to socket io to something that allows to keep state per client. [(GH-72)](http://github.com/scalatra/scalatra/issues/72)

project/build.scala

@@ -68,6 +68,11 @@ object ScalatraBuild extends Build {
           <name>Ivan Porto Carrero</name>
           <url>http://flanders.co.nz/</url>
         </developer>
+        <developer>
+          <id>jlarmstrong</id>
+          <name>Jared Armstrong</name>
+          <url>http://www.jaredarmstrong.name/</url>
+        </developer>
       </developers>
     )},
     publishTo <<= (version) { version: String =>
@@ -96,21 +101,21 @@ object ScalatraBuild extends Build {
 
     val commonsFileupload = "commons-fileupload" % "commons-fileupload" % "1.2.1"
 
-    val commonsIo = "commons-io" % "commons-io" % "1.4"
+    val commonsIo = "commons-io" % "commons-io" % "2.0.1"
 
-    private def jettyDep(name: String) = "org.eclipse.jetty" % name % "7.4.1.v20110513"
+    private def jettyDep(name: String) = "org.eclipse.jetty" % name % "7.4.5.v20110725"
     val testJettyServlet = jettyDep("test-jetty-servlet")
     val jettyWebsocket = jettyDep("jetty-websocket")
     val jettyWebapp = jettyDep("jetty-webapp")
 
-    val junit = "junit" % "junit" % "4.8.1"
+    val junit = "junit" % "junit" % "4.8.2"
 
     def liftJson(scalaVersion: String) = {
       val libArtifactId = scalaVersion match {
         case "2.9.1.RC1" => "lift-json_2.9.0"
         case x => "lift-json_"+x
       }
-      "net.liftweb" % libArtifactId % "2.4-M2"
+      "net.liftweb" % libArtifactId % "2.4-M3"
     }
 
     val scalamd = "org.fusesource.scalamd" % "scalamd" % "1.5"
@@ -122,7 +127,6 @@ object ScalatraBuild extends Build {
     def scalatest(scalaVersion: String) = {
       val libArtifactId = scalaVersion match {
         case "2.9.1.RC1" => "scalatest_2.9.0"
-        case "2.9.0-1" => "scalatest_2.9.0"
         case x => "scalatest_"+x
       }
       "org.scalatest" % libArtifactId % "1.6.1"