dom96 / jester

A sinatra-like web framework for Nim.
MIT License
1.58k stars 120 forks source link

MultiData parsing doesn't handle preamble properly #86

Open nigredo-tori opened 8 years ago

nigredo-tori commented 8 years ago

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 commented 8 years ago

Is this not just a Nim stdlib issue?

nigredo-tori commented 8 years ago

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

dom96 commented 7 years ago

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

nigredo-tori commented 7 years ago

I can confirm as well that the initial example works. I suspect it was fixed on client side by this PR: https://github.com/nim-lang/Nim/pull/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).