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

Support non-UTF8 requests #11

Open
owickstrom opened this issue Dec 20, 2016 · 6 comments
Open

Support non-UTF8 requests #11

owickstrom opened this issue Dec 20, 2016 · 6 comments

Comments

@owickstrom
Copy link
Collaborator

owickstrom commented Dec 20, 2016

The String instance of readBody in the Node server assumes UTF8, it should check Content-Type header.

@rightfold
Copy link
Contributor

rightfold commented May 15, 2017

It should also support binary data, and streaming, and MIME in general. I think the fundep on ReadableBody needs to be removed for this.

@owickstrom
Copy link
Collaborator Author

@rightfold Agreed. Not sure where you mean the functional dependency is, though?

@rightfold
Copy link
Contributor

Fundep from body reader type to body type.

class ReadableBody req m b | req -> b

@owickstrom
Copy link
Collaborator Author

Yes, you're absolutely right. I'll try loosening that, and adding some more instances.

Regarding binary, I could add an instance for getting an Aff e Buffer, as with String.

For streaming, exposing the request's Stream will do for now. In the spirit of Hyper, the stream should only be allowed to be read once (it cannot be read twice, but you could get a compile-time error if you try). That would probably require some other design than just handing back the Readable stream -- like hiding the streaming within the indexed monad and track whether you've been streaming or not. Any thoughts?

@rightfold
Copy link
Contributor

Well, where "read once" means "read until exhausted." Stream has a godforsaken Node.js-style callback API, so you can't really demand to read it twice, right? It'll just stop calling you when there's nothing to read.

@owickstrom
Copy link
Collaborator Author

Regarding this issue, it should only be about the String instance checking the Content-Type header and using that when doing toString on the buffer. Buffers in general, and streaming, are separate issues, but we already have the PRs open. 👍

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

No branches or pull requests

2 participants