Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Submitting a form with an empty file field throws exception #149

Open
nikosk opened this issue Nov 11, 2024 · 2 comments · May be fixed by #150
Open

Submitting a form with an empty file field throws exception #149

nikosk opened this issue Nov 11, 2024 · 2 comments · May be fixed by #150

Comments

@nikosk
Copy link

nikosk commented Nov 11, 2024

Minimal repro:

case class FormRoute()(implicit
    cc: castor.Context,
    log: cask.Logger
) extends cask.MainRoutes:

  @cask.get("/")
  def index() =
    Response("""
    <!DOCTYPE html>
    <html lang="en">
    <head></head>
    <body>
        <form action="/post" method="post" enctype="multipart/form-data">    
            <input type="file" id="image" name="image" accept="image/*">
            <button type="submit">Submit</button>
        </form>
    </body>
    </html>
    """, 200, Seq(("Content-Type", "text/html")))    

  @cask.postForm("/post")
  def post(image: cask.FormFile) = s"Image filename: ${image.fileName}"

  initialize()


object Application extends cask.Main:
    val allRoutes = Seq(FormRoute())

Stacktrace:

Unable to parse form data: java.lang.IllegalStateException: UT000017: Form value is a file, use getFileItem() instead
java.lang.IllegalStateException: UT000017: Form value is a file, use getFileItem() instead
	at io.undertow.server.handlers.form.FormData$FormValueImpl.getValue(FormData.java:351)
	at cask.model.FormEntry$.fromUndertow(Params.scala:102)

The method cask.model.FormEntry#fromUndertow is naively implemented and does not take into account optional field values. Browsers AFAIK will always send fields even if they are empty, it's up to the server/app to decide what emptiness means.
At the very least I would expect fromUndertow to return Option[FormEntry] and let the postForm decorator deal with it.

Below is an example request from Firefox, but I would expect all browsers to behave the same.

-----------------------------265322345236697440593672026017
Content-Disposition: form-data; name="image"; filename=""
Content-Type: application/octet-stream


-----------------------------265322345236697440593672026017--

I tried to take a stab at fixing this but the project doesn't compile (tried all tags as low as 0.9.0) and the error messages are too cryptic for me (scala.reflect.internal.FatalError: bad constant pool index: 0 at pos: 48454 wtf?!). Let me know if you want me to open a ticket for that also.
@lihaoyi
Copy link
Member

lihaoyi commented Nov 11, 2024

thanks for the report. For developing the Mill repo, it might be a Java versikn issue. Can you try with Java 11

@nikosk nikosk linked a pull request Nov 12, 2024 that will close this issue
Open
@nikosk
Copy link
Author

nikosk commented Nov 12, 2024

The only user facing change is that now FormFile.filePath is optional. I think it's a good compromise instead of forcing users to expect Option[FormFile] parameters in their endpoints.

I was debating whether fileName should also be optional. I chose to let it accept null values, happy to change it someone thinks otherwise.

case class FormFile(fileName: String,
                    filePath: Option[java.nio.file.Path], <-- users will see this
                    headers: io.undertow.util.HeaderMap) extends FormEntry{
  def valueOrFileName = fileName
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fixes form submissions with empty file fields throwing exceptions nikosk/cask
2 participants
@nikosk @lihaoyi