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.

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

Specifying multipart/form-data content type does not include the boundary in request headers #169

Closed
julian-baldwin opened this issue Mar 22, 2024 · 6 comments

Comments

@julian-baldwin
Copy link

This is an issue I discovered when using Dexador to send multipart/form-data content to a server that contained some form fields but not a file input (for test purposes). Dexador correctly generates a boundary value and encodes the form content, however does not include the boundary in the headers.

To demonstrate in Lisp:

CL-USER 1 > (ql:quickload "dexador")
CL-USER 2 > (dex:post "http://localhost:8080/"
                      :headers '((:content-type . "multipart/form-data"))
                      :content '(("field" . "value")))

Request as received by the server (in a shell):

% nc -l 127.0.0.1 8080
POST / HTTP/1.1
User-Agent: Dexador/0.9.15 (LispWorks 8.0.1); Darwin; 22.6.0
Host: localhost:8080
Accept: */*
Content-Type: multipart/form-data
Content-Length: 89

--Yr9qnF9d6Xnn
Content-Disposition: form-data; name="field"

value
--Yr9qnF9d6Xnn--

At least for the usocket backend this arises due to the local function write-header* in dexador.backend.usocket:request using the value in the function's arguments rather than the value computed in the headers-data local function. Generating and specifying a boundary value in the function arguments also fails as Dexador generates a new one and uses that to encode the body.

Probably the simplest fix is to remove the Content-Type header from the list of headers supplied as function arguments if processing multipart/form-data content. An alternative fix would be to extract the boundary from the arguments and use that to encode the body instead of generating a new one, although that strikes me as comparatively quite involved and I can't imagine why a caller would want to compute their own boundary instead of letting Dexador do it.

I am happy to submit a PR if this is of interest, although I don't know if/how this affects the winhttp backend and cannot test it.

julian-baldwin added a commit to julian-baldwin/dexador that referenced this issue Mar 22, 2024
…Length headers in preference to caller-supplied ones.
@ajberkley
Copy link
Collaborator

Thank you for the bug report and suggested fix. I will take a look!

@ajberkley
Copy link
Collaborator

Your fix looks correct to me, please create a PR from your fork. I believe the winhttp backend does the right thing. The bug was introduced in 138b08a in Feb 2023. @fukamachi does this make sense to you? (If you are too busy to respond, don't worry, I will merge the PR after some more poking around).

@ajberkley
Copy link
Collaborator

Actually no need to submit a PR. I added a test for this case anyway, so will make the changes! Thank you again for the bug report!

@ajberkley
Copy link
Collaborator

#170

I am just checking windows behavior (the CI is broken because of some package cl-isaac not available?)

@ajberkley
Copy link
Collaborator

#170 fixes this --- thank you for your bug report and the fix (I added a test for this behavior)

@julian-baldwin
Copy link
Author

#170 fixes this --- thank you for your bug report and the fix (I added a test for this behavior)

No problems at all, thanks for the fix! :)

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

No branches or pull requests

2 participants