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

WAI 2.0 #178

Closed
kazu-yamamoto opened this issue Oct 3, 2013 · 30 comments
Closed

WAI 2.0 #178

kazu-yamamoto opened this issue Oct 3, 2013 · 30 comments

Comments

@kazu-yamamoto
Copy link
Contributor

This issue is to discuss WAI 2.0 (the internal-state branch). Please read http://www.haskell.org/pipermail/web-devel/2013/002646.html first.

@kazu-yamamoto
Copy link
Contributor Author

Recent ByteString provides its own Builder. But WAI 2.0 uses Builder provided in blaze-buidler. This is intentional because old HP does not include recent ByteString.

The new logging code implemented by me depends on blaze-builder. So, this decision is also fine with me.

@kazu-yamamoto
Copy link
Contributor Author

isSecure is intentionally retained. @snoyberg would you wrap up the discussion on this?

@kazu-yamamoto
Copy link
Contributor Author

Currently, WAI 2.0 does not provide serverName and serverPort. How can a WAI application get such information?

To get serverName, the Host: header field is not enough because HTTP 1.0 does not provide Host:.
I have no idea to obtain a local port.

wai-app-file-cgi uses both to specify SERVER_NAME and SERVER_PORT to GCI and to re-organize original URL to redirect "http://host:port/dir" to "http://host:port/dir/" (the last slash is important).

@snoyberg
Copy link
Member

snoyberg commented Oct 3, 2013

If you look at the current version of parseRequest', the serverName is not being set in any intelligent manner right now for HTTP 1.0 requests. It's just being set to "". As for serverPort, a WAI app can't reliably know that information, at least not in the way you're trying to use it, since:

  1. It may be working on a protocol that doesn't inform it of the port.
  2. There may not be a port (e.g., wai-test), though that's a corner case we can ignore.
  3. Reverse proxying could make it that the port a user is connecting on is different than the port the request is received on.

The approach I took for this in Yesod is to require the user to provide the application root, which addresses this issue as well as determining whether a request was on a secure connection or not.

@kazu-yamamoto
Copy link
Contributor Author

@snoyberg Can you find the issue in which we discussed support for HTTP 1.0? I cannot find it but should read it to tell whether or not HTTP 1.0 support is still necessary at this moment.

@snoyberg
Copy link
Member

snoyberg commented Oct 3, 2013

I don't remember discussing HTTP 1.0, and searching the issue tracker produced no results.

@kazu-yamamoto
Copy link
Contributor Author

Uhhm. What do you think of HTTP 1.0 support? Do you think we can drop it?

@snoyberg
Copy link
Member

snoyberg commented Oct 3, 2013

I don't think we need to, there will just be some specific cases (like a missing host header) that will cause some applications to not work correctly. I don't think there's anything in Warp that is actually depending on HTTP 1.1.

Also, looking at the host header spec, it seems like your redirecting would be better handled by just using the raw host header, since that is required to have the port number if it's different than port 80. This doesn't address the HTTPS issue, but it's better than the current usage of serverPort, which is known to be broken in the presence of reverse proxying.

@kazu-yamamoto
Copy link
Contributor Author

Did you mean "not need to support" or "not need to drop"?

Thank you for letting me know aboutt the Host header!

@snoyberg
Copy link
Member

snoyberg commented Oct 3, 2013

I meant that we don't need to explicitly drop any support in Warp; I think its current behavior works well for HTTP 1.0, even if there are some corner cases which will cause problems for specific applications.

@kazu-yamamoto
Copy link
Contributor Author

OK. Thanks. I agree.

@kazu-yamamoto
Copy link
Contributor Author

@snoyberg I'm trying to catch up the internal-state branch of wai/warp in wai-app-file-cgi. To do this, I made the internal-state2 branch:

https://github.com/kazu-yamamoto/wai-app-file-cgi/tree/internal-state2

I cannot resolve two things. So, I need your advice.

In cgiApp' (line 51), register is used. I don't know how to register something to internalState. How can I do it?

In http (line 96), a function of http-conduit is used. Should I wait that http-conduit is ready for WAI 2.0?

@snoyberg
Copy link
Member

snoyberg commented Oct 3, 2013

For register, it should go something like this:

runInternalState (register yourAction) (resourceInternalState req)

This might give you an idea of how to use http-conduit:

https://github.com/fpco/http-reverse-proxy/blob/wai-2.0/Network/HTTP/ReverseProxy.hs#L227

But I think it would make sense to start a version of http-conduit that isn't dependent on ResourceT, and thus clean up these complications. That was part of my motivation for starting the http-client that I mentioned in a different ticket.

@iand675
Copy link
Contributor

iand675 commented Oct 3, 2013

One question I have about blaze-builder's Builder monoid vs bytestring's: what kind of performance gains would there be from utilizing bytestring Builder's ability to be written directly to a handle? Assuming that the Haskell Platform release timetable is followed, bytestring's Builder ought to be available in the new Haskell Platform in November.

@gregorycollins
Copy link

Uhhm. What do you think of HTTP 1.0 support? Do you think we can drop it?

The HTTP/1.1 spec says that you have to support it.

@snoyberg
Copy link
Member

snoyberg commented Oct 3, 2013

@iand675 In theory, we'd get no benefits, since Warp writes directly to sockets, not handles.

@snoyberg
Copy link
Member

snoyberg commented Oct 3, 2013

@gregorycollins Good point.

@singpolyma
Copy link
Contributor

What decision was made regarding exporting/not exporting the Request constructor? It seems to me that removing pattern matching as an option is a pretty big deal for such a small gain.

@snoyberg
Copy link
Member

snoyberg commented Oct 3, 2013

It's being exported from the .Internal module.

@singpolyma
Copy link
Contributor

@snoyberg Ok. I guess I can just import .Internal in all of my apps...

@kazu-yamamoto
Copy link
Contributor Author

@iand675 My new logger (which is much faster than fast-logger) does not use Handle. Handle uses MVar which becomes a giant lock under multicore IO manager (Mio). You can find my new logger in https://github.com/kazu-yamamoto/mighttpd2/tree/ver3 at this moment. And new magic is in https://github.com/kazu-yamamoto/mighttpd2/blob/ver3/Program/Mighty/LogMsg.hs. I will extract these files to make a new library.

@kazu-yamamoto
Copy link
Contributor Author

@singpolyma Constructors are hide by default so that we can modify internals with less impact.

@singpolyma
Copy link
Contributor

@kazu-yamamoto if you only add fields, the impact is minimal anyway. If you remove them, that's going to affect users either way.

@kazu-yamamoto
Copy link
Contributor Author

@singpolyma You are right. :-)

@kazu-yamamoto
Copy link
Contributor Author

@snoyberg Thanks. I succeeded to modify CGI.hs. I don't know how to modify RevProxy.hs. However, it's better to wait for your new package. Meanwhile, I will do yield hack, removing lookup in Warp, profiling Warp, and creating a new logger library.

@snoyberg
Copy link
Member

snoyberg commented Oct 4, 2013

Awesome. I'll continue my conduit research/benchmarks and work on http-client a bit more. I'll probably send an email to web-devel about it.

@kazu-yamamoto
Copy link
Contributor Author

@gregorycollins Which section of RFC 2616 does say so? If you refer to 19.6, we need to implement 0.9, too. :-(

@kazu-yamamoto
Copy link
Contributor Author

I will study HTTP/2.0, too. http://tools.ietf.org/html/draft-ietf-httpbis-http2-06

@gregorycollins
Copy link

@kazu-yamamoto I seem to remember reading that but can't find it in the spec now. In any event dropping support for HTTP/1.0 is not really feasible, there are clients that still use it :(

@kazu-yamamoto
Copy link
Contributor Author

@gregorycollins, I agreed @snoyberg with not dropping HTTP/1.0 support. I just wanted which section of RFC 2616 says so. Also, I have no plan to support HTTP/0.9. But I will try HTTP/2.0. :-)

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

5 participants