net: http: Add compression support in HTTP server #86490
Conversation
kica-z
force-pushed
the
feature/http-server-compression-support
branch
from
c247ea7 to
f0533cf
Compare
There was a problem hiding this comment.
No objection to the feature, just wondering if it would make sense to have this more configurable. I mean that if user only needs gzip support, then the other methods are useless to compile in. Perhaps have a Kconfig entries for each supported compression method, and then compile out code that is not needed.
kica-z
force-pushed
the
feature/http-server-compression-support
branch
from
f0533cf to
70779c4
Compare
|
@jukkar I agree in the sense that a kconfig to enable compression in general would be good. I am however not sure if kconfigs for each compression method would't be to much as the only thing they would remove is one I will also have a look at the tests that are failing (need to add tests covering compression anyhow). |
Add compression support using the accept-encoding header to the http server static filesystem resource. Signed-off-by: Carlo Kirchmeier <carlo.kirchmeier@zuehlke.com>
kica-z
force-pushed
the
feature/http-server-compression-support
branch
from
70779c4 to
7271508
Compare
| @@ -201,6 +201,21 @@ config WEBSOCKET_CONSOLE | |||
| help | |||
| Hidden option that is enabled only when all the necessary options | |||
| needed by websocket console are set. | |||
|
|
|||
| config HTTP_SERVER_COMPRESSION | |||
| bool "Compression support in http fileserver" | |||
There was a problem hiding this comment.
| bool "Compression support in http fileserver" | |
| bool "Compression support in HTTP fileserver" |
| HTTP_ZSTD = 4 /**< ZSTD */ | ||
| }; | ||
|
|
||
| void http_compression_parse_accept_encoding(const char *accept_encoding, size_t len, |
There was a problem hiding this comment.
I just wonder, does it need to be a public header, those efunctions seem rather internal?
There was a problem hiding this comment.
And also the functions should be documented using doxygen comments.
| return ""; | ||
| } | ||
|
|
||
| int http_compression_from_text(enum http_compression compression[static 1], const char *text) |
There was a problem hiding this comment.
Why not just enum http_compression *compression? TBH it's a bit confusing to see an 1-element array being passed here, plus, is static 1 even a valid C syntax, or just some GCC extension? First time I see a construct like this being used.
Same goes to other functions.
| if (ret < 0) { | ||
| len = strlen(fname); | ||
| len = strlen(fname); | ||
| #ifdef CONFIG_HTTP_SERVER_COMPRESSION |
There was a problem hiding this comment.
Can we use if (IS_ENABLED(CONFIG_HTTP_SERVER_COMPRESSION)) { ... } whenever possible. Not only this makes code compile unconditionally, but should also allow to avoid extra ifdef slicing, like around return_filename label.
What you have now (one Kconfig option) looks fine. |
Add compression support using the accept-encoding header to the http server static filesystem resource.
In our use case we have different files with different compressions it is therefore important that the webserver is able to handle those compressions correctly. This is a first draft of the feature. I am open to discussion on how to implement this or if this is even wanted in upstream.