Awesome, thanks! I reviewed and tested and all looks fine. I don't want to bother you but I see two improvings:
1. While ETag minimizes load on each load will be sent a new request. To avoid such requests at all for assets we can add a header Cache-Control: public,max-age=31536000,immutable But users may want some other values so it would be great to have a general ability to add a custom header. This can be implemented just as a regular httpd.conf file in a folder with the value of a custom header. The same can be used for Content Security Policy (CSP) headers and many other things. Some users are looking for the functionality https://serverfault.com/questions/918602/how-to-set-header-with-busybox-httpd Will you accept a patch for custom headers? 2. BB httpd has a built-in IP ACL i.e. allow/deny by IP. This work can be done by a firewall or iptables. We can also make it configurable: Subject: [PATCH] httpd: Make Deny/Allow by IP config support optional Signed-off-by: Sergey Ponomarev <stok...@gmail.com> --- networking/httpd.c | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/networking/httpd.c b/networking/httpd.c index 0297cf9ec..7ddf6caaa 100644 --- a/networking/httpd.c +++ b/networking/httpd.c @@ -245,6 +245,13 @@ //config: help //config: RFC2616 says that server MUST add Date header to response. //config: But it is almost useless and can be omitted. +//config: +//config:config FEATURE_HTTPD_ACL +//config: bool "ACL IP" +//config: default y +//config: depends on HTTPD +//config: help +//config: Deny/Allow by IP config support //applet:IF_HTTPD(APPLET(httpd, BB_DIR_USR_SBIN, BB_SUID_DROP)) @@ -314,6 +321,7 @@ typedef struct Htaccess { char before_colon[1]; /* really bigger, must be last */ } Htaccess; +#if ENABLE_FEATURE_HTTPD_ACL_IP /* Must have "next" as a first member */ typedef struct Htaccess_IP { struct Htaccess_IP *next; @@ -321,6 +329,7 @@ typedef struct Htaccess_IP { unsigned mask; int allow_deny; } Htaccess_IP; +#endif /* Must have "next" as a first member */ typedef struct Htaccess_Proxy { @@ -449,7 +458,9 @@ struct globals { const char *found_mime_type; const char *found_moved_temporarily; +#if ENABLE_FEATURE_HTTPD_ACL_IP Htaccess_IP *ip_a_d; /* config allow/deny lines */ +#endif IF_FEATURE_HTTPD_BASIC_AUTH(const char *g_realm;) IF_FEATURE_HTTPD_BASIC_AUTH(char *remoteuser;) @@ -499,7 +510,9 @@ struct globals { #define found_mime_type (G.found_mime_type ) #define found_moved_temporarily (G.found_moved_temporarily) #define last_mod (G.last_mod ) -#define ip_a_d (G.ip_a_d ) +#if ENABLE_FEATURE_HTTPD_ACL_IP +# define ip_a_d (G.ip_a_d ) +#endif #define g_realm (G.g_realm ) #define remoteuser (G.remoteuser ) #define file_size (G.file_size ) @@ -560,6 +573,7 @@ static ALWAYS_INLINE void free_Htaccess_list(Htaccess **pptr) free_llist((has_next_ptr**)pptr); } +#if ENABLE_FEATURE_HTTPD_ACL_IP static ALWAYS_INLINE void free_Htaccess_IP_list(Htaccess_IP **pptr) { free_llist((has_next_ptr**)pptr); @@ -649,6 +663,7 @@ static int scan_ip_mask(const char *str, unsigned *ipp, unsigned *maskp) *maskp = (uint32_t)(~mask); return 0; } +#endif /* * Parse configuration file into in-memory linked list. @@ -677,8 +692,10 @@ static void parse_conf(const char *path, int flag) const char *filename; char buf[160]; +#if ENABLE_FEATURE_HTTPD_ACL_IP /* discard old rules */ free_Htaccess_IP_list(&ip_a_d); +#endif flg_deny_all = 0; /* retain previous auth and mime config only for subdir parse */ if (flag != SUBDIR_PARSE) { @@ -783,6 +800,7 @@ static void parse_conf(const char *path, int flag) continue; } +#if ENABLE_FEATURE_HTTPD_ACL_IP if (ch == 'A' || ch == 'D') { Htaccess_IP *pip; @@ -819,6 +837,7 @@ static void parse_conf(const char *path, int flag) } continue; } +#endif #if ENABLE_FEATURE_HTTPD_ERROR_PAGES if (flag == FIRST_PARSE && ch == 'E') { @@ -1920,6 +1939,7 @@ static NOINLINE void send_file_and_exit(const char *url, int what) log_and_exit(); } +#if ENABLE_FEATURE_HTTPD_ACL_IP static void if_ip_denied_send_HTTP_FORBIDDEN_and_exit(unsigned remote_ip) { Htaccess_IP *cur; @@ -1949,6 +1969,7 @@ static void if_ip_denied_send_HTTP_FORBIDDEN_and_exit(unsigned remote_ip) if (flg_deny_all) /* depends on whether we saw "D:*" */ send_headers_and_exit(HTTP_FORBIDDEN); } +#endif #if ENABLE_FEATURE_HTTPD_BASIC_AUTH @@ -2228,7 +2249,9 @@ static void handle_incoming_and_exit(const len_and_sockaddr *fromAddr) if (verbose > 2) bb_simple_error_msg("connected"); } +#if ENABLE_FEATURE_HTTPD_ACL_IP if_ip_denied_send_HTTP_FORBIDDEN_and_exit(remote_ip); +#endif /* Install timeout handler. get_line() needs it. */ signal(SIGALRM, send_REQUEST_TIMEOUT_and_exit); @@ -2378,7 +2401,9 @@ static void handle_incoming_and_exit(const len_and_sockaddr *fromAddr) if (is_directory(urlcopy + 1, /*followlinks:*/ 1)) { /* may have subdir config */ parse_conf(urlcopy + 1, SUBDIR_PARSE); +#if ENABLE_FEATURE_HTTPD_ACL_IP if_ip_denied_send_HTTP_FORBIDDEN_and_exit(remote_ip); +#endif } *tptr = '/'; } -- 2.25.1 On Sun, 16 Aug 2020 at 00:57, Denys Vlasenko <vda.li...@googlemail.com> wrote: > I applied patch 4 as well with some edits. > Please test current git. > Thank you. > > On Sat, Aug 15, 2020 at 11:05 PM Denys Vlasenko > <vda.li...@googlemail.com> wrote: > > > > On Sun, Aug 9, 2020 at 12:24 AM Sergey Ponomarev <stok...@gmail.com> > wrote: > > > > > > If server respond with ETag then next time client (browser) resend it > via If-None-Match header. > > > Then httpd will check if file wasn't modified and if not return 304 > Not Modified status code. > > > The ETag value is constructed from file's last modification date in > unix epoch and it's size: > > > "hex(last_mod)-hex(file_size)" e.g. "5e132e20-417" (with quotes). > > > That means that it's not completely reliable as hash functions but > fair enough. > > > The same form of ETag is used by Nginx so load balancing of static > content is safe. > > > > > > Signed-off-by: Sergey Ponomarev <stok...@gmail.com> > > > --- > > > networking/httpd.c | 73 ++++++++++++++++++++++++++++++++++++++++++++-- > > > 1 file changed, 70 insertions(+), 3 deletions(-) > > > > > > diff --git a/networking/httpd.c b/networking/httpd.c > > > index 1cea33ddd..cc3f757aa 100644 > > > --- a/networking/httpd.c > > > +++ b/networking/httpd.c > > > @@ -215,6 +215,17 @@ > > > //config: Makes httpd send files using GZIP content encoding if > the > > > //config: client supports it and a pre-compressed <file>.gz > exists. > > > //config: > > > +//config:config FEATURE_HTTPD_ETAG > > > +//config: bool "Support caching via ETag header" > > > +//config: default y > > > +//config: depends on HTTPD > > > +//config: help > > > +//config: If server respond with ETag then next time client > (browser) resend it via If-None-Match header. > > > +//config: Then httpd will check if file wasn't modified and if > not return 304 Not Modified status code. > > > +//config: The ETag value is constructed from file's last > modification date in unix epoch and it's size: > > > +//config: "hex(last_mod)-hex(file_size)" e.g. "5e132e20-417" > (with quotes). > > > +//config: That means that it's not completely reliable as hash > functions but fair enough. > > > > $ make > > GEN networking/Config.in > > scripts/kconfig/conf -s Config.in > > networking/Config.in:294 error: Overlong line > > networking/Config.in:295 error: Overlong line > > networking/Config.in:296 error: Overlong line > > networking/Config.in:298 error: Overlong line > > networking/Config.in:306 error: Overlong line > > networking/Config.in:307 error: Overlong line > > # > > # using defaults found in .config > > > > > > > +//config: > > > //config:config FEATURE_HTTPD_LAST_MODIFIED > > > //config: bool "Add Last-Modified header to response" > > > //config: default y > > > @@ -266,6 +277,7 @@ > > > > > > #include "libbb.h" > > > #include "common_bufsiz.h" > > > +#include <inttypes.h> > > > > libbb.h includes inttypes.h > > > > > +/* > > > + * ETag is "hex(last_mod)-hex(file_size)" e.g. "5e132e20-417" > > > + */ > > > +static char *make_etag(void) > > > +{ > > > + return xasprintf("\"%" PRIx64 "-%" PRIx64 "\"", last_mod, > file_size); > > > +} > > > + > > > > networking/httpd.c: In function 'make_etag': > > networking/httpd.c:1082:19: error: format '%llx' expects argument of > > type 'long long unsigned int', but argument 2 has type 'time_t {aka > > long int}' [-Werror=format=] > > return xasprintf("\"%" PRIx64 "-%" PRIx64 "\"", last_mod, file_size); > > ^~~~~ > -- Sergey Ponomarev <https://linkedin.com/in/stokito>, skype:stokito
_______________________________________________ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
