On Fri, Nov 30, 2018 at 10:16:45AM +0100, Florian Obser wrote:
> On Fri, Oct 26, 2018 at 03:08:11PM -0600, Tracey Emery wrote:
> > On Mon, Jul 30, 2018 at 10:24:03AM -0600, Base Pr1me wrote:
> > > Sorry, this time with the correct diff.
> > > 
> > > On 7/25/18 4:15 PM, Base Pr1me wrote:
> > > > Hi,
> > > > 
> > > > I discovered that the wrong server configuration is evaluated in the
> > > > server_read_http function. Only the first server in httpd.conf is 
> > > > checked. For
> > > > example, I have five servers setup in httpd.conf and the third server 
> > > > is the
> > > > only one with connection { max request body ####} set, because I desire 
> > > > it to
> > > > accept larger uploads than the other servers. When the upload is 
> > > > initiated,
> > > > server one dictates the max request body size, globally.
> > > > 
> > > > The attached diff moves the queue loop out of the server_response 
> > > > function in to
> > > > its own function, as to not duplicate code.
> > > > 
> > > > I don't know if this is the only place the wrong information is 
> > > > evaluated. Also,
> > > > I'm not sure this is the best method to fix the problem, but it should 
> > > > point the
> > > > powers that be in the right direction.
> > > > 
> > > > Thanks,
> > > > 
> > > > Tracey
> > > > 
> > > 
> > 
> > Hello,
> > 
> > I reworked the last sent diff. I missed fixing up the hostname. This was 
> > causing
> > an incorrect 301 on urls not containing an ending slash. I also moved the
> > srv_conf assignment into the new function.
> > 
> > Again, this is to use the correct server config information from the queue 
> > for
> > server_read_http and remove code duplication.
> > 
> > If anyone is willing to look at this and make suggestions, that'd be great. 
> > If
> > not, that'd be great too! LOL. Have a great weekend.
> > 
> > Thanks,
> > Tracey
> 
> (moving back from bugs@)
> 
> Sorry for slacking of on this for so long and thanks for prodding
> patiently :)
> 
> The issue is that in server_read_http we haven't found the correct
> virtual server & location yet.
> clt->clt_srv_conf contains the server_config struct for the listening IP.
> 
> But there is no need to check maxrequestbody just yet, we can do it in
> server_response() where we do know the virtual host.
> 
> Tracey, can you please test this, it should solve your problem.
> 
> OK?
> 
> diff --git server_http.c server_http.c
> index e05cec56dfc..52698a66b2e 100644
> --- server_http.c
> +++ server_http.c
> @@ -198,7 +198,6 @@ void
>  server_read_http(struct bufferevent *bev, void *arg)
>  {
>       struct client           *clt = arg;
> -     struct server_config    *srv_conf = clt->clt_srv_conf;
>       struct http_descriptor  *desc = clt->clt_descreq;
>       struct evbuffer         *src = EVBUFFER_INPUT(bev);
>       char                    *line = NULL, *key, *value;
> @@ -357,11 +356,6 @@ server_read_http(struct bufferevent *bev, void *arg)
>                               server_abort_http(clt, 500, errstr);
>                               goto abort;
>                       }
> -                     if ((size_t)clt->clt_toread >
> -                         srv_conf->maxrequestbody) {
> -                             server_abort_http(clt, 413, NULL);
> -                             goto abort;
> -                     }
>               }
>  
>               if (strcasecmp("Transfer-Encoding", key) == 0 &&
> @@ -1334,6 +1328,12 @@ server_response(struct httpd *httpd, struct client 
> *clt)
>               srv_conf = server_getlocation(clt, desc->http_path);
>       }
>  
> +     if (clt->clt_toread > 0 && (size_t)clt->clt_toread >
> +         srv_conf->maxrequestbody) {
> +             server_abort_http(clt, 413, NULL);
> +             return (-1);
> +     }
> +
>       if (srv_conf->flags & SRVFLAG_BLOCK) {
>               server_abort_http(clt, srv_conf->return_code,
>                   srv_conf->return_uri);
> 
> 
> 
> -- 
> I'm not entirely sure you are real.

Thank you, sir. That does, indeed, fix the maxrequestbody issue.

As an aside, when doing verbose debugging, the log still shows the first virtual
server when doing the upload. It's not as an important issue as the request
body, but certainly something to be aware of when you have the time to look.
Naturally, I've changed domain names below! ;)

www.upload.vs 192.168.1.2 - - [30/Nov/2018:08:01:40 -0700] "GET 
/support/file_upload.php?iss_id=917 HTTP/1.1" 200 0
www.upload.vs 192.168.1.2 - - [30/Nov/2018:08:01:47 -0700] "POST 
/support/ajax/upload.php?file=dropfile HTTP/1.1" 200 0
# wrong virt serv #
server first_vs_in_httpd.conf, client 1 (1 active), 192.168.1.2:15244 -> 
10.0.0.10:443, closed
www.upload.vs 192.168.1.2 - - [30/Nov/2018:08:01:51 -0700] "POST 
/support/file_upload.php HTTP/1.1" 200 0
www.upload.v 192.168.1.2 - - [30/Nov/2018:08:01:53 -0700] "GET 
/support/view.php?id=917 HTTP/1.1" 200 0

Thanks,
Tracey

Reply via email to