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

MultiData parsing doesn't handle preamble properly #86

Open
nigredo-tori opened this issue Oct 5, 2016 · 4 comments
Open

MultiData parsing doesn't handle preamble properly #86

nigredo-tori opened this issue Oct 5, 2016 · 4 comments
Labels

Comments

@nigredo-tori
Copy link
Contributor

httpclient overhaul triggered this bug in Jester. It now crashes on multipart message with non-empty preamble part.
Example code:

import jester, httpclient, asyncdispatch, tables
from nativesockets import Port
from os import sleep

proc runServer() {.thread.} =
  settings:
    port = Port(1234)
    bindAddr = "127.0.0.1"

  routes:
    post "/mp":
      let fd = request.formData
      assert fd.hasKey("foo")
      resp("OK")

  runForever()

  echo "done"

var t: Thread[void]
createThread(t, runServer)

sleep(1000)

let url = "http://127.0.0.1:1234/mp"
var md = newMultipartData()
md.add(
  name = "foo",
  content = "xyz"
)

echo "This works:"
echo postContent(url, multipart = md)

let client = newHttpClient()
echo "This crashes:"
echo client.postContent(url, multipart = md)

This fails with the following error message:

Traceback (most recent call last)
test_multipart_jester.nim(37) test_multipart_jester
httpclient.nim(1109)     postContent
httpclient.nim(1088)     post
httpclient.nim(1037)     request
httpclient.nim(1021)     request
httpclient.nim(938)      parseResponse
httpclient.nim(154)      httpError
Error: unhandled exception: Connection was closed before full request has been made [ProtocolError]

Here's the reason: old (deprecated) and new postContent versions differ in HTTP they output; in particular the new version's output has an extra empty line before the first boundary. This line should be considered part of the preamble (as specified in RFC1341), and ignored.

@dom96
Copy link
Owner

dom96 commented Oct 5, 2016

Is this not just a Nim stdlib issue?

@nigredo-tori
Copy link
Contributor Author

Since Jester fails for valid HTTP, I'd say it's Jester issue in the first place.

nigredo-tori added a commit to vegansk/nimboost that referenced this issue Oct 5, 2016
@dom96
Copy link
Owner

dom96 commented May 17, 2017

I can no longer reproduce this, has it been fixed?

@nigredo-tori
Copy link
Contributor Author

I can confirm as well that the initial example works. I suspect it was fixed on client side by this PR: nim-lang/Nim#5711 . Jester still seems to choke on preamble, though:

Server:

import jester, asyncdispatch
from nativesockets import Port

settings:
  port = Port(1234)
  bindAddr = "127.0.0.1"

routes:
  post "/mp":
    let fd = request.formData
    assert fd.hasKey("foo")
    resp("OK")

runForever()

Http request:

POST /mp HTTP/1.1
Host: localhost
Content-Type: multipart/form-data; boundary=XX
Content-Length: 66


--XX
Content-Disposition: form-data; name="foo"

bar
--XX--

Sending this makes the server close the connection, though the program itself doesn't crash like before (at least on Windows).

@dom96 dom96 added the Bug label May 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants