Skip to content

Commit cbe92cd

Browse files
authored
Fixes form submissions with empty file fields throwing exceptions (#150)
Given a form with a file filed i.e. `<input type="file"/>` if the user does not include a file to upload when the form is submitted the browser will send a request with the following multipart body: ``` -----------------------------101075975012956453003083734809 Content-Disposition: form-data; name="some-field" -----------------------------101075975012956453003083734809 Content-Disposition: form-data; name="a-file"; filename="" Content-Type: application/octet-stream -----------------------------101075975012956453003083734809-- ``` Since there is no file but there is a file field, the browser will send a part with empty filename and body and octet-stream as the content type to signify a submission without a file to upload. In cask to handle such form submissions you create an endpoint like the following: ``` @cask.postForm("/post") def post(image: cask.FormFile) = s"Image filename: ${image.fileName}" ``` The `postForm` decorator upon receiving the submission it will create an Undertow `MultiPartParserDefinition` and try to parse the request. After parsing the boundary and the headers of the part Undertow will then try to parse the body which is empty and because there is nothing to parse it will never create a file for the `FormValue` representing the empty file field (see `MultiPartParserDefinition` line 329): ![image](https://github.com/user-attachments/assets/cbfa167f-a870-413a-9f48-9890595ca042) This results to `FormData` with a `FormValue` where the `fileitem` field is `null`. ![image](https://github.com/user-attachments/assets/6fc890ba-6ae9-40be-9ab3-a5c7fd7e74fc) So when cask tries to convert the Undertow `FormValue`s to cask `FormEntry` by calling `FormEntry.fromUndertow` on each FormValue the `FormValue.isFile` condition returns false and cask tries to convert the Undertow `FormValue` to a cask `FormValue` and an exception is thrown when calling `getValue()` on a binary `FormValue`. ![image](https://github.com/user-attachments/assets/0f7715a4-c166-4dd2-b8f1-a2c3efb0bd10) ![image](https://github.com/user-attachments/assets/c69f804a-e54e-49d8-adf8-ce550a2f96e0) ![image](https://github.com/user-attachments/assets/e00b0328-c1b9-4037-98d5-2747c1ae2211) In essence if your endpoint is using the `postForm` decorator and is expecting a `FormField` cask will throw an exception if the user doesn't submit a file and there's no chance for the developer to do any validation. This PR is preventing the exception from being thrown by detecting the empty binary field and replicating the Undertow behaviour of passing a `FormValue` with null values (it's converted to Option[Path] in cask land) so that the code has a chance to perform validation and accept or reject the form submission. ``` def fromUndertow(from: io.undertow.server.handlers.form.FormData.FormValue): FormEntry = { val isOctetStream = Option(from.getHeaders) .flatMap(headers => Option(headers.get(HttpString.tryFromString("Content-Type")))) .exists(h => h.asScala.exists(v => v == "application/octet-stream")) // browsers will set empty file fields to content type: octet-stream if (isOctetStream || from.isFileItem) FormFile(from.getFileName, Try(from.getFileItem.getFile).toOption, from.getHeaders) else FormValue(from.getValue, from.getHeaders) } ``` Closes #149 --------- Co-authored-by: nk <nk>
1 parent 0b14e42 commit cbe92cd

File tree

6 files changed

+92
-5
lines changed

6 files changed

+92
-5
lines changed

build.mill

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,7 @@ def zippedExamples = T {
125125
build.example.websockets2.millSourcePath,
126126
build.example.websockets3.millSourcePath,
127127
build.example.websockets4.millSourcePath,
128+
build.example.multipartFormSubmission.millSourcePath,
128129
)
129130

130131
for (example <- examples) yield {

cask/src/cask/model/Params.scala

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,12 @@
11
package cask.model
22

33
import java.io.{ByteArrayOutputStream, InputStream}
4-
54
import cask.internal.Util
65
import io.undertow.server.HttpServerExchange
76
import io.undertow.server.handlers.CookieImpl
7+
import io.undertow.util.HttpString
8+
import scala.util.Try
9+
import scala.collection.JavaConverters.collectionAsScalaIterableConverter
810

911
case class QueryParams(value: Map[String, collection.Seq[String]])
1012
case class RemainingPathSegments(value: Seq[String])
@@ -98,9 +100,13 @@ sealed trait FormEntry{
98100
}
99101
}
100102
object FormEntry{
101-
def fromUndertow(from: io.undertow.server.handlers.form.FormData.FormValue) = {
102-
if (!from.isFile) FormValue(from.getValue, from.getHeaders)
103-
else FormFile(from.getFileName, from.getPath, from.getHeaders)
103+
def fromUndertow(from: io.undertow.server.handlers.form.FormData.FormValue): FormEntry = {
104+
val isOctetStream = Option(from.getHeaders)
105+
.flatMap(headers => Option(headers.get(HttpString.tryFromString("Content-Type"))))
106+
.exists(h => h.asScala.exists(v => v == "application/octet-stream"))
107+
// browsers will set empty file fields to content type: octet-stream
108+
if (isOctetStream || from.isFileItem) FormFile(from.getFileName, Try(from.getFileItem.getFile).toOption, from.getHeaders)
109+
else FormValue(from.getValue, from.getHeaders)
104110
}
105111

106112
}
@@ -110,7 +116,9 @@ case class FormValue(value: String,
110116
}
111117

112118
case class FormFile(fileName: String,
113-
filePath: java.nio.file.Path,
119+
filePath: Option[java.nio.file.Path],
114120
headers: io.undertow.util.HeaderMap) extends FormEntry{
115121
def valueOrFileName = fileName
116122
}
123+
124+
case class EmptyFormEntry()

example/multipartFormSubmission/app/resources/example.txt

Whitespace-only changes.
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
package app
2+
3+
object MultipartFormSubmission extends cask.MainRoutes {
4+
5+
@cask.get("/")
6+
def index() =
7+
cask.model.Response(
8+
"""
9+
<!DOCTYPE html>
10+
<html lang="en">
11+
<head></head>
12+
<body>
13+
<form action="/post" method="post" enctype="multipart/form-data">
14+
<input type="file" id="somefile" name="somefile">
15+
<button type="submit">Submit</button>
16+
</form>
17+
</body>
18+
</html>
19+
""", 200, Seq(("Content-Type", "text/html")))
20+
21+
@cask.postForm("/post")
22+
def post(somefile: cask.FormFile) =
23+
s"filename: ${somefile.fileName}"
24+
25+
initialize()
26+
}
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
package app
2+
import io.undertow.Undertow
3+
4+
import utest._
5+
6+
object ExampleTests extends TestSuite{
7+
def withServer[T](example: cask.main.Main)(f: String => T): T = {
8+
val server = Undertow.builder
9+
.addHttpListener(8081, "localhost")
10+
.setHandler(example.defaultHandler)
11+
.build
12+
server.start()
13+
val res =
14+
try f("http://localhost:8081")
15+
finally server.stop()
16+
res
17+
}
18+
19+
val tests = Tests {
20+
test("MultipartFormSubmission") - withServer(MultipartFormSubmission) { host =>
21+
val withFile = requests.post(s"$host/post", data = requests.MultiPart(
22+
requests.MultiItem("somefile", Array[Byte](1,2,3,4,5) , "example.txt"),
23+
))
24+
withFile.text() ==> s"filename: example.txt"
25+
withFile.statusCode ==> 200
26+
27+
val withoutFile = requests.post(s"$host/post", data = requests.MultiPart(
28+
requests.MultiItem("somefile", Array[Byte]()),
29+
))
30+
withoutFile.text() ==> s"filename: null"
31+
withoutFile.statusCode ==> 200
32+
}
33+
}
34+
}
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
package build.example.multipartFormSubmission
2+
import mill._, scalalib._
3+
4+
object app extends Cross[AppModule](build.scalaVersions)
5+
trait AppModule extends CrossScalaModule{
6+
7+
def moduleDeps = Seq(build.cask(crossScalaVersion))
8+
9+
def ivyDeps = Agg[Dep](
10+
)
11+
object test extends ScalaTests with TestModule.Utest{
12+
13+
def ivyDeps = Agg(
14+
ivy"com.lihaoyi::utest::0.8.4",
15+
ivy"com.lihaoyi::requests::0.9.0",
16+
)
17+
}
18+
}

0 commit comments

Comments
 (0)