[prev in list] [next in list] [prev in thread] [next in thread] 

List:       busybox
Subject:    Re: [PATCH 4/4] httpd: Support caching via ETag header
From:       Denys Vlasenko <vda.linux () googlemail ! com>
Date:       2020-08-16 13:40:20
Message-ID: CAK1hOcNzY4xVMk5s-e50v_QusW09SqnKEsmth5u45PBhEyOp1g () mail ! gmail ! com
[Download RAW message or body]

Applied, thanks

On Sun, Aug 16, 2020 at 12:49 AM Sergey Ponomarev <stokito@gmail.com> wrote:
> 
> 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 <stokito@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.linux@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.linux@googlemail.com> wrote:
> > > 
> > > On Sun, Aug 9, 2020 at 12:24 AM Sergey Ponomarev <stokito@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 <stokito@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, skype:stokito
_______________________________________________
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic