-
Notifications
You must be signed in to change notification settings - Fork 12
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
handling of route #60
Comments
Do you still use a GADT for typing paths? wrt the tree? Otherwise: I don't like removing the url_encode choice. Something you may want a raw string, and sometimes not. For the most part I'm worried I'll have to modify my existing projects that use tiny_httpd… |
Le 13 février 2023 04:46:55 GMT-10:00, Simon Cruanes ***@***.***> a écrit :
Do you still use a GADT for typing paths? wrt the tree?
I mostly kept your code. I just added a tree structure to handle method and exact prefix.
You can have a look at simple_httpd and steal what you like.
Otherwise: I don't like removing the url_encode choice. Something you may want a raw string, and sometimes not. For the most part I'm worried I'll have to modify my existing projects that use tiny_httpd…
I was considering an optimisation of parsing the first line of request as I did for
The others. Then, it will include URL decode.
By the way, I now have an ADT for header names generated from a CSV from iana.
Like this I caught the issue in the test.
Now I only alocate once the header value ( I integrate the trim in parsing). Your code allocated three times the line of each header: The full line, the header name before and after lowercase conv and the header value before and after trim. Simplifying this did improve speed a lot.
…-- >
Reply to this email directly or view it on GitHub:
#60 (comment)
You are receiving this because you authored the thread.
Message ID: ***@***.***>
|
For common headers? That's pretty cool. |
Serious question: at what point is simple_httpd just a different project, anyway? It feels like you're doing a lot of work on it, it's faster, it leverages OCaml 5, etc. Maybe it's simpler to make simple_httpd an official project of yours? |
I decided to accept only the headers listed at iana. The are so many that I am sure one can find one with a reasonable name if you need something.
Le 13 février 2023 06:35:45 GMT-10:00, Simon Cruanes ***@***.***> a écrit :
…> By the way, I now have an ADT for header names generated from a CSV from iana.
For common headers? That's pretty cool.
--
Reply to this email directly or view it on GitHub:
#60 (comment)
You are receiving this because you authored the thread.
Message ID: ***@***.***>
|
Le 13 février 2023 06:37:16 GMT-10:00, Simon Cruanes ***@***.***> a écrit :
Serious question: at what point is simple_httpd just a different project, anyway? It feels like you're doing a lot of work on it, it's faster, it leverages OCaml 5, etc. Maybe it's simpler to make simple_httpd an official project of yours?
Yes, it is what I think I did. But I still feel like reporting bugs and suggestions for tiny_httpd, as we still share a lot of code.
|
You mean, you don't accept custom headers at all? :o Thank you for the reports. Indeed, the ones about chunk encoding, etc. are very useful. I need to work on them. |
Simon Cruanes writes:
You mean, you don't accept custom headers at all? :o
Yes I don't. I have never add any use for them (contrary to custom
cookies), and if you accept any custom header, then you do not catch
spelling mistake in the frontend code (like for the test).
In fact I want to propose some (simple) parser combinator for input
buffers, as a ppx, not only for speed of parsing, but to put the header
values in a record and avoid the assoc list. I will give a default
parser for headers that can be extended both for the server and each
route.
This way you can ignore the header you don't use, but still check they
are valid and I can accept custom headers and have a clear error
in the log if there is an invalid header. THat't the plan, but I will
probably release simple_httpd without this feature.
Thank you for the reports. Indeed, the ones about chunk encoding,
etc. are very useful. I need to work on them.
The spec on Accept-encoding and Content-encoding are a bit strange.
Accept-encoding only mention some compression algorithm and not chunked
or identity. This means all clients and servers must accept
chunked-encoding I guess? So the current code is probably ok (I checked
that after reporting the issue) and only the test should be corrected,
putting no header (and not Accept-encoding chunked which appears to be invalid).
I think chunked encoding should be reserved to the case where it is
costly or impossible to compute the body length. So there is probably no
way for the client to force the server to send chunked encoded body?
…--
Christophe Raffalli
tél: +689 87 23 11 48
web: http://raffalli.eu
|
Is it compliant to reject custom headers? Or do you mean you just throw the parsing result away and move on to the next line? I actually like the chunked encoding. If your body is a stream, it's just the cleaner way to go. And clients should be able to process either fixed size or chunked bodies anyway. |
Simon Cruanes writes:
Is it compliant to _reject_ custom headers? Or do you mean you just throw the parsing result away and move on to the next line?
I did not think about that. I will do what you suggest: rejecting the
line and continue + message in the log. Currently I was issuing bad
request, but your idea is better.
I actually like the chunked encoding. If your body is a stream, it's
just the cleaner way to go. And clients should be able to process
either fixed size or chunked bodies anyway.
Yes I like it too. And at last I found a clear paragraph in the
spec:
"""
7.4. Negotiating Transfer Codings
The TE field (Section 10.1.4 of [HTTP]) is used in HTTP/1.1 to indicate
what transfer codings, besides chunked, the client is willing to accept
in the response and whether the client is willing to preserve trailer
fields in a chunked transfer coding.
A client MUST NOT send the chunked transfer coding name in TE; chunked
is always acceptable for HTTP/1.1 recipients. ...
"""
Still they mention TE which seems to be redundant with Accept-Encoding.
I understand this as "the server can use chunked or content-length and the client
as no way to choose what the servor will do contrary to compressions".
…--
Christophe Raffalli
tél: +689 87 23 11 48
web: http://raffalli.eu
|
Hi @craff, I know it's been a while, but I've been revisiting issues :). I'd be interested in some parts of this actually!
for the use of combinators, I'd be interested in seeing what they look like/how they're implemented but I think it belongs in a separate discussion. |
I have made several improvements (?) to route handling in simple_httpd you might want to use for tiny_httpd:
The text was updated successfully, but these errors were encountered: