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

HTTP/2 support #132

Open
bapcyk opened this issue Feb 3, 2023 · 8 comments
Open

HTTP/2 support #132

bapcyk opened this issue Feb 3, 2023 · 8 comments

Comments

@bapcyk
Copy link

bapcyk commented Feb 3, 2023

The function parseStatusLine expects "HTTP/1.X" but we have already "HTTP/2". For me, I get Parse exception: not enough input, not sure the cause, but maybe this function is a problem:

parseStatusLine :: Parser (Int, ByteString)
parseStatusLine = do
    sc <- string "HTTP/1." *> satisfy version *> char ' ' *> decimal <* char ' '
    sm <- takeTill (== '\r') <* crlf
    return (sc, sm)
  where
    version c = c == '1' || c == '0'

?

@istathar
Copy link
Member

istathar commented Feb 6, 2023

I think you'll find that supporting HTTP 2.0 is a much bigger topic than just changing the parser. I'd like it very much if we could! We probably have to deal with this fairly soon.

@bapcyk
Copy link
Author

bapcyk commented Feb 6, 2023

@istathar Do you have idea what is the reason of the error Parse exception: not enough input? Is it really due to string "HTTP/1."?

@istathar
Copy link
Member

istathar commented Feb 6, 2023

@bapcyk The diagnostics (sic) out of attoparsec are so rubbish. I didn't know better in 2013 when I was writing this; nowadays I would optimize for comprehensible error messages over some notional and arguable idea of "performance".

Do you have a way I can see the error you're seeing (is the URL public)?

@bapcyk
Copy link
Author

bapcyk commented Feb 6, 2023

@bapcyk The diagnostics (sic) out of attoparsec are so rubbish. I didn't know better in 2013 when I was writing this; nowadays I would optimize for comprehensible error messages over some notional and arguable idea of "performance".

Do you have a way I can see the error you're seeing (is the URL public)?

Unfortunately the URL is not public.
I saw a header like:

< HTTP/2 302
< location: https://cdn.XYZ.com/api/get/21338/serve?nva=20230202134343&token=XXXXXXXXXXXXXXXXXXXX
< x-rate-limit-duration: 1
< x-rate-limit-limit: 5.00
< x-rate-limit-request-forwarded-for: 192.168.1.101, 192.168.1.102
< x-rate-limit-request-remote-addr: 192.168.1.103:16418
< date: Fri, 03 Feb 2023 13:03:43 GMT
< content-length: 0
< via: 1.1 google
< alt-svc: h3=":443"; ma=2592000,h3-29=":443"; ma=2592000

(curl output) and supposed that the problem maybe is in it.

@istathar
Copy link
Member

istathar commented Feb 6, 2023

Well you're right it's trying to upgrade to an HTTP/2 connection. It's a radically different protocol. We're not in a particularly good place in Haskell client-wise there (ironically, we have an excellent HTTP/2 server in the http2 package). See there, and the [separate] http2-client package. I don't know how well along either of these things are, but HTTP 2.0 is complicated. I have a feeling we'd all be better off supporting their work.

@istathar
Copy link
Member

istathar commented Feb 6, 2023

(fwiw at one site this http-streams package is handling 100 million requests [POSTs, in fact] a day no problem. So it's working fine. I just don't know what our upgrade path is)

@bapcyk
Copy link
Author

bapcyk commented Feb 6, 2023

Ah, well, OK then. I will switch to another package currently.
Maybe to close the issue then?

@istathar
Copy link
Member

istathar commented Feb 7, 2023

Maybe to close the issue then?

Nah, leave it open. It's something we need to work out.

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

2 participants