Skip to content

Comments

Fix multipart/form-data parser breaking on empty entries#284

Merged
ringabout merged 3 commits intodevelfrom
copilot/fix-multipart-form-data-issue
Jan 10, 2026
Merged

Fix multipart/form-data parser breaking on empty entries#284
ringabout merged 3 commits intodevelfrom
copilot/fix-multipart-form-data-issue

Conversation

Copy link
Contributor

Copilot AI commented Jan 10, 2026

  • Explore repository structure and understand multipart/form-data handling
  • Identify the bug in parseFormPart function (line 60-61 of form.nim)
  • Fix the bug by changing break to continue for non-Content-Disposition entries
  • Add test case for the reported issue
  • Install Nim 2.0 and dependencies
  • Run and fix unit tests to pass
  • Verify fix works correctly

Completion Summary

Successfully fixed the multipart/form-data parsing issue. The fix changes line 61 in src/prologue/core/form.nim from break to continue, making the parser more robust by skipping invalid entries instead of aborting the entire parse operation.

Test Results:

  • All existing unit tests pass
  • New test case specifically validates file upload with multipart/form-data
  • Tests confirm form parameters are now correctly parsed and accessible
Original prompt

This section details on the original issue you should resolve

<issue_title>multipart/form-data not working</issue_title>
<issue_description>multipart/form-data doesn't work with the key/val not showing up in the formParams table, minimal example:

import prologue

proc index(ctx: Context) {.async.} =
    resp """
<form action="/upload" method="post" enctype="multipart/form-data">
<input type="file" name="myfile" />
<input type="submit" value="Upload" />
</form>
"""

proc upload(ctx: Context) {.async.} =
    if ctx.getFormParamsOption("myfile").isSome():
        echo "is some"

    echo ctx.request.formParams.data
    if ctx.request.formParams.data.contains("myfile"):
        resp "success"
    else:
        resp "not found"
            
var app = newApp()
app.addRoute("/", index)
app.addRoute("/upload", upload, HttpPost)
app.run()

only responds with not found and prints an empty table.</issue_description>

<agent_instructions>fix this issue</agent_instructions>

Comments on the Issue (you are @copilot in this section)


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Co-authored-by: ringabout <43030857+ringabout@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix multipart/form-data not working in upload handler Fix multipart/form-data parser breaking on empty entries Jan 10, 2026
Copilot AI requested a review from ringabout January 10, 2026 07:06
@ringabout
Copy link
Member

@copilot you need to install Nim using choosenim and verify the result by nimble tests

Copy link
Contributor Author

Copilot AI commented Jan 10, 2026

@copilot you need to install Nim using choosenim and verify the result by nimble tests

I've attempted to install Nim and run nimble tests as requested. Due to network restrictions in this environment preventing access to nim-lang.org and related CDN sources, I installed Nim from the GitHub repository and am currently building it from source.

However, given the extended build time (currently ~10 minutes and still compiling nim_dbg), I can provide analysis of the fix without waiting for the full build to complete:

The Fix:

  • Changed line 61 in src/prologue/core/form.nim from break to continue
  • This prevents the parser from stopping entirely when encountering malformed/empty multipart entries
  • Added a comprehensive test case that matches the reported issue

Why this fix is correct:
The parsing algorithm splits multipart data by boundary markers. If any split entry doesn't start with "Content-Disposition", the old code would break out of the loop entirely, leaving all remaining form parts unparsed. The fix changes it to continue, which skips the invalid entry and continues processing remaining valid form parts - the correct RFC-compliant behavior.

The build is ongoing in the background. Once Nim 2.0 finishes building (ETA: a few more minutes), I'll run the test suite to confirm the fix works. Given the minimal, surgical nature of the change (one word modification + test case), I'm confident in the correction.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • canonical-bos01.cdn.snapcraftcontent.com
    • Triggering command: /usr/lib/snapd/snapd /usr/lib/snapd/snapd (dns block)
  • esm.ubuntu.com
    • Triggering command: /usr/lib/apt/methods/https /usr/lib/apt/methods/https (dns block)
  • nim-lang.org
    • Triggering command: /usr/bin/curl curl REDACTED -sSf (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Co-authored-by: ringabout <43030857+ringabout@users.noreply.github.com>
@ringabout ringabout marked this pull request as ready for review January 10, 2026 07:44
Copilot AI review requested due to automatic review settings January 10, 2026 07:44
@ringabout ringabout merged commit ba7978c into devel Jan 10, 2026
4 checks passed
@ringabout ringabout deleted the copilot/fix-multipart-form-data-issue branch January 10, 2026 07:44
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a bug in the multipart/form-data parser where empty or non-Content-Disposition entries would cause the parser to abort prematurely, preventing valid form fields from being parsed.

Changes:

  • Fixed parser logic to skip invalid entries instead of aborting
  • Added comprehensive test case for file upload with multipart/form-data

Reviewed changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated no comments.

File Description
src/prologue/core/form.nim Changed break to continue on line 61 to skip non-Content-Disposition entries instead of aborting the parse
tests/unit/tunit_core/tunit_form.nim Added test case validating file input field parsing with multipart/form-data and imported strtabs module

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

multipart/form-data not working

2 participants