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

Feature/bodies #71

Merged
merged 72 commits into from May 27, 2015
Merged

Feature/bodies #71

merged 72 commits into from May 27, 2015

Conversation

haf
Copy link
Owner

@haf haf commented Apr 5, 2015

A PR based on the previous two PRs, aiming to fix #65

@haf
Copy link
Owner Author

haf commented Apr 6, 2015

this PR is the one I really care about ;). Or the stuff that starts from the first [requestbody] commit to be specific.

@haf
Copy link
Owner Author

haf commented Apr 6, 2015

Rebasing against master.

@relentless
Copy link
Collaborator

I'll just wait until you're done then. Will this include the Fuchu changes as well?

@haf
Copy link
Owner Author

haf commented Apr 6, 2015

Yes

@haf
Copy link
Owner Author

haf commented Apr 6, 2015

New issue after rebase: #82

@haf
Copy link
Owner Author

haf commented Apr 6, 2015

Rebase done. Afk for a while now. You should be able to test this.

@relentless
Copy link
Collaborator

Not sure what's going on here, there seems to be some issue with the Fsharp.core reference in the unit tests, and the integration tests hang at some point.

@relentless
Copy link
Collaborator

The fsharp.core redirect in the unit test project needs updating to 4.3.1. I'm getting two failures now, related to the multi-part file upload:

formatting different sorts of body/can format multipart/formdata single file: Failed:
should have correct body
Expected: "Content-Type: multipart/form-data; boundary=mACKqCcIID-J''_PL:hfbFiOLC/cew

--mACKqCcIID-J''_PL:hfbFiOLC/cew
Content-Disposition: form-data; name="submit-name"

Larry
--mACKqCcIID-J''_PL:hfbFiOLC/cew
Content-Disposition: form-data; name="files"; filename="file1.txt"
Content-Type: text/plain

Hello World
--mACKqCcIID-J''_PL:hfbFiOLC/cew--"
Actual: "Content-Type: multipart/form-data; boundary=nLWsTCFurKCiU_PjC/cCmmU-tnJHHa

--nLWsTCFurKCiU_PjC/cCmmU-tnJHHa
Content-Disposition: form-data; name="submit-name"

Larry
--nLWsTCFurKCiU_PjC/cCmmU-tnJHHa
Content-Disposition: form-data; name="files"; filename="file1.txt"
Content-Type: text/plain

Hello World
--nLWsTCFurKCiU_PjC/cCmmU-tnJHHa--"
g:\prg\Fuchu\Fuchu\Assertions.fs(13,1): <StartupCode$Fuchu>.$Assertions.Equal@13.Invoke(String msg)
(00:00:00.0334506)

formatting different sorts of body/can format multipart/formdata with multipart/mixed for multi-file upload: Failed:
should have correct body
Expected: "Content-Type: multipart/form-data; boundary=mACKqCcIID-J''_PL:hfbFiOLC/cew

--mACKqCcIID-J''_PL:hfbFiOLC/cew
Content-Disposition: form-data; name="submit-name"

Larry
--mACKqCcIID-J''_PL:hfbFiOLC/cew
Content-Type: multipart/mixed; boundary=iDnsCZhfTqMSYsj:LhBTftNfVog:eA
Content-Disposition: form-data; name="files"

--iDnsCZhfTqMSYsj:LhBTftNfVog:eA
Content-Disposition: file; filename="file1.txt"
Content-Type: text/plain

Hello World
--iDnsCZhfTqMSYsj:LhBTftNfVog:eA
Content-Disposition: file; filename="file2.gif"
Content-Type: text/plain

...contents of file2.gif...
--iDnsCZhfTqMSYsj:LhBTftNfVog:eA--
--mACKqCcIID-J''_PL:hfbFiOLC/cew--"
Actual: "Content-Type: multipart/form-data; boundary=nLWsTCFurKCiU_PjC/cCmmU-tnJHHa

--nLWsTCFurKCiU_PjC/cCmmU-tnJHHa
Content-Disposition: form-data; name="submit-name"

Larry
--nLWsTCFurKCiU_PjC/cCmmU-tnJHHa
Content-Type: multipart/mixed; boundary=BgOE:fCUQGnYfKwGMnxoyfwVMbRzZF
Content-Disposition: form-data; name="files"

--BgOE:fCUQGnYfKwGMnxoyfwVMbRzZF
Content-Disposition: file; filename="file1.txt"
Content-Type: text/plain

Hello World
--BgOE:fCUQGnYfKwGMnxoyfwVMbRzZF
Content-Disposition: file; filename="file2.gif"
Content-Type: text/plain

...contents of file2.gif...
--BgOE:fCUQGnYfKwGMnxoyfwVMbRzZF--
--nLWsTCFurKCiU_PjC/cCmmU-tnJHHa--"
g:\prg\Fuchu\Fuchu\Assertions.fs(13,1): <StartupCode$Fuchu>.$Assertions.Equal@13.Invoke(String msg)

@relentless
Copy link
Collaborator

Also, the integration tests hang on this:

***** HttpClient_SuaveIntegrationTests+Suave Integration Tests.server receives valid filenames
get response body
[I] 2015-04-06T21:47:59.5908870Z: listener started in 97.069 ms with binding 127.0.0.1:8083 [Suave.Tcp.tcp_ip_server]
[V] 2015-04-06T21:48:00.6253196Z: 127.0.0.1 connected, total: 1 clients [Suave.Tcp.tcp_ip_server.job]
[V] 2015-04-06T21:48:00.6323246Z: reserving buffer: 811008, free count: 99 [Suave.Tcp.tcp_ip_server.job] [Suave.Socket.BufferManager]
[V] 2015-04-06T21:48:00.6423316Z: -> processor [Suave.Web.request_loop.loop]
[V] 2015-04-06T21:48:00.6633465Z: reserving buffer: 802816, free count: 98 [Suave.Web.read_till_pattern.loop] [Suave.Socket.BufferManager]
[V] 2015-04-06T21:48:00.7053906Z: freeing buffer: 802816, free count: 99 [Suave.Web.split] [Suave.Socket.BufferManager]
[V] 2015-04-06T21:48:00.7364162Z: reserving buffer: 802816, free count: 98 [Suave.Web.read_till_pattern.loop] [Suave.Socket.BufferManager]
[V] 2015-04-06T21:48:00.9904504Z: tcp request processing failed [Suave.Tcp.tcp_ip_server.job] exn:
System.Exception: did not find header, because header_params received None
at Suave.Utils.Parsing.headerParams(FSharpOption1 header) at Suave.Web.ParsingAndControl.loop@207-31.Invoke(Tuple2 _arg2)
at Microsoft.FSharp.Control.AsyncBuilderImpl.args@787-1.Invoke(a a)

@haf
Copy link
Owner Author

haf commented Apr 7, 2015

The first error is interesting; because it means Random(seed) has different implementations on mono vs .Net

@haf
Copy link
Owner Author

haf commented Apr 7, 2015

So it turns out RFC1867 that I coded against isn't actually implemented by web servers, and all web servers simply repeat the form-name for multiple files. Gaah.

http://stackoverflow.com/questions/16976299/how-to-create-an-html-form-to-send-http-message-with-multipart-mixed-mime-type/29487399#29487399

@haf
Copy link
Owner Author

haf commented Apr 7, 2015

@haf
Copy link
Owner Author

haf commented Apr 7, 2015

Wow, what a yak.

At least now there are passing integration tests =). Ok, please review again.

@haf
Copy link
Owner Author

haf commented May 20, 2015

Now we have integration tests for sending a binary file, too that are passing with the latest Suave.

@haf
Copy link
Owner Author

haf commented May 20, 2015

Latest suave https://twitter.com/SuaveIO/status/601018318280126464 -- works on my machine =)

@haf
Copy link
Owner Author

haf commented May 20, 2015

I have updated the namespaces here, too.

@haf
Copy link
Owner Author

haf commented May 20, 2015

So on Windows, on AppVeyor, something is obviously deadlocking / locking the async and that never returns. Since the web server runs to completion, sending all its data, I'm a bit surprised. Any insights?

@haf
Copy link
Owner Author

haf commented May 21, 2015

Some feedback now that all issues have been solved would be great!

@relentless
Copy link
Collaborator

Good work, nice green builds! I'm due a 'learning day' at work, so I'll take that soon and try to go through all you've done. (Although there's a lot to go through!)

haf added 5 commits May 22, 2015 18:49
This refactor closes almost all outstanding issues.

 - Introduces a Logging abstraction, fixes #80
 - Previously: not version Releases -- fixes #78
 - Previously: explicit state -- fixes #77
 - Uses Map which has something like O(log n) -- fixes 76
 - Username/password in UTF8 - fixes #73
 - Fixes #72 by defaulting everything to UTF-8
 - Fixes #67 by overwriting headers set n times
 - Previously: Fixes #65 about form bodies
 - Sets Expect100Continue to false, since we've overridden the
   GetResponseStream() interaction and always send all of the request.
 - Cleans up indexing into response.Headers to be functional instead of
   imperative
 - Improved docs on BodyForm
@haf
Copy link
Owner Author

haf commented May 27, 2015

Request for merge!

relentless added a commit that referenced this pull request May 27, 2015
@relentless relentless merged commit 9d0ab0a into haf:master May 27, 2015
@relentless
Copy link
Collaborator

OK, have merged. Still want to check a few things, and want to go through it before making a NuGet.

@haf haf deleted the feature/bodies branch May 27, 2015 12:50
@haf
Copy link
Owner Author

haf commented May 27, 2015

Yes, you should -- I absolutely want feedback and to reach consensus. How about we create issues for each discussion point, and you can cc @haf?

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.

[Body] Support the different sorts of bodies
3 participants