belyalov / tinyweb

Simple and lightweight HTTP async server for micropython
MIT License
239 stars 40 forks source link

force lowercase headers and force uppercase method #54

Closed eyJhb closed 2 months ago

eyJhb commented 1 year ago

Fixes issue #53 as well as forcing methods to be uppercase. Comments are welcome!

dydek commented 1 year ago

Hi @eyJhb , I've found the same issue lately ( eg using JS fetch with custom headers gives you the mixed headers with title and lowercase style) - Don't you think that the method https://github.com/belyalov/tinyweb/blob/master/tinyweb/server.py#L134 should also be changed? I solved the issue differently, by using the same ( and consistent ) lower-cased headers names ( eg in the method mentioned above ) across the source code and changing the https://github.com/belyalov/tinyweb/blob/master/tinyweb/server.py#L99 to provide also lowercased headers.

I can help write some additional tests to ensure everything is working as expected with all the needed cases, but I'm not so sure if @belyalov still maintains this project.

richardcockerill commented 2 months ago

I experienced a variation on this problem too regarding Content-Length and Content-Type on PUT and POST requests when using vite development server while developing a client side frontend with a tinyweb backend. The vite dev server proxy mutated the headers to lower case before forwarding to my tiny web backend and the headers failed to match and the request bodies were not read. I had to fix the string literals at https://github.com/belyalov/tinyweb/blob/master/tinyweb/server.py#L134 and https://github.com/belyalov/tinyweb/blob/master/tinyweb/server.py#L137 too

Worthing noting that the http standard says that head fields names are case insensitive https://datatracker.ietf.org/doc/html/rfc1945#section-4.2

Each header field consists of a name followed immediately by a colon (":"), a single space (SP) character, and the field value. Field names are case-insensitive.

eyJhb commented 2 months ago

@FabianClemenz any chance this could be merged?

belyalov commented 2 months ago

Thanks, it makes sense to me.