On Tue, Nov 28, 2023 at 05:57:39AM +0300, Maxim Dounin wrote:
> Hello!
>
> On Fri, Nov 10, 2023 at 12:11:55PM +0300, Vladimir Homutov via nginx-devel
> wrote:
>
> >
> > It is no longer used since the refactoring in 8e5bf1bc87e2 (2008).
>
> Neither r->port_start nor r->port_end were ever used.
>
> The r->port_end is set by the parser, though it was never used by
> the following code (and was never usable, since not copied by the
> ngx_http_alloc_large_header_buffer() without r->port_start set).
>
> The 8e5bf1bc87e2 commit is completely unrelated, it is about
> refactoring of the ngx_parse_inet_url() function, which had a
> local variable named "port_start".
exactly, thanks for noticing.
>
> >
> > diff --git a/src/http/ngx_http_request.c b/src/http/ngx_http_request.c
> > --- a/src/http/ngx_http_request.c
> > +++ b/src/http/ngx_http_request.c
> > @@ -1744,8 +1744,7 @@ ngx_http_alloc_large_header_buffer(ngx_h
> > }
> > }
> >
> > - if (r->port_start) {
> > - r->port_start = new + (r->port_start - old);
> > + if (r->port_end) {
> > r->port_end = new + (r->port_end - old);
> > }
> >
> > diff --git a/src/http/ngx_http_request.h b/src/http/ngx_http_request.h
> > --- a/src/http/ngx_http_request.h
> > +++ b/src/http/ngx_http_request.h
> > @@ -597,7 +597,6 @@ struct ngx_http_request_s {
> > u_char *schema_end;
> > u_char *host_start;
> > u_char *host_end;
> > - u_char *port_start;
> > u_char *port_end;
> >
> > unsigned http_minor:16;
>
> I don't think it's a good change. Rather, we should either remove
> both, or (eventually) fix these and provide some valid usage of
> the port as parsed either from the request line or from the Host
> header, similarly to the $host variable.
>
I think that we should remove both, as unused code still needs to be
maintained without any advantage, as this example shows.
Restoring it will be trivial, if ever required.
# HG changeset patch
# User Vladimir Khomutov <v...@wbsrv.ru>
# Date 1701165434 -10800
# Tue Nov 28 12:57:14 2023 +0300
# Node ID dacad3a9c7b8435a4c67ad2b67f261e7b4e36d8e
# Parent f366007dd23a6ce8e8427c1b3042781b618a2ade
HTTP: removed unused r->port_start and r->port_end.
Neither r->port_start nor r->port_end were ever used.
The r->port_end is set by the parser, though it was never used by
the following code (and was never usable, since not copied by the
ngx_http_alloc_large_header_buffer() without r->port_start set).
diff --git a/src/http/ngx_http_parse.c b/src/http/ngx_http_parse.c
--- a/src/http/ngx_http_parse.c
+++ b/src/http/ngx_http_parse.c
@@ -451,19 +451,16 @@ ngx_http_parse_request_line(ngx_http_req
switch (ch) {
case '/':
- r->port_end = p;
r->uri_start = p;
state = sw_after_slash_in_uri;
break;
case '?':
- r->port_end = p;
r->uri_start = p;
r->args_start = p + 1;
r->empty_path_in_uri = 1;
state = sw_uri;
break;
case ' ':
- r->port_end = p;
/*
* use single "/" from request line to preserve pointers,
* if request line will be copied to large client buffer
diff --git a/src/http/ngx_http_request.c b/src/http/ngx_http_request.c
--- a/src/http/ngx_http_request.c
+++ b/src/http/ngx_http_request.c
@@ -1735,11 +1735,6 @@ ngx_http_alloc_large_header_buffer(ngx_h
}
}
- if (r->port_start) {
- r->port_start = new + (r->port_start - old);
- r->port_end = new + (r->port_end - old);
- }
-
if (r->uri_ext) {
r->uri_ext = new + (r->uri_ext - old);
}
diff --git a/src/http/ngx_http_request.h b/src/http/ngx_http_request.h
--- a/src/http/ngx_http_request.h
+++ b/src/http/ngx_http_request.h
@@ -597,8 +597,6 @@ struct ngx_http_request_s {
u_char *schema_end;
u_char *host_start;
u_char *host_end;
- u_char *port_start;
- u_char *port_end;
unsigned http_minor:16;
unsigned http_major:16;
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel
