On Mon, Jun 8, 2020 at 9:30 PM Ruediger Pluem <[email protected]> wrote:
>
> On 6/8/20 4:59 PM, Yann Ylavic wrote:
> > On Mon, Jun 8, 2020 at 9:56 AM Ruediger Pluem <[email protected]> wrote:
> >>
> >> I came across the question if we should not reject HTTP protocols >= 2.0
> >> in the request line when we parse it
> >> in ap_parse_request_line.
> >
> > Why not >= 1.2 ?
> >
> >> A possible patch could look like the following (which rejects such
> >> requests with a HTTP_VERSION_NOT_SUPPORTED status code):
> >
> > Looks good.
> >
> >
> > In the same category, could we reject invalid URI paths earlier
> > (request-target per RFC-7230 5.3)?
> > Today it fails in ap_core_translate(), but possibly the below would be
> > better:
>
> I think we could, but I am not sure if we have ap_parse_uri callers in other
> parts of the code that do not pass absolute URI's
This patch works with absolute URIs too since apr_parse_uri will split
it in r->parsed_uri and we consider r->parsed_uri.path only.
>
> >
> > Index: server/protocol.c
> > ===================================================================
> > --- server/protocol.c (revision 1878328)
> > +++ server/protocol.c (working copy)
> > @@ -627,6 +627,14 @@ AP_CORE_DECLARE(void) ap_parse_uri(request_rec *r,
> > }
> > else {
> > status = apr_uri_parse(r->pool, uri, &r->parsed_uri);
> > + if (status == APR_SUCCESS
> > + && (r->parsed_uri.path != NULL)
> > + && (r->parsed_uri.path[0] != '/')
> > + && (r->method_number != M_OPTIONS
> > + || strcmp(r->parsed_uri.path, "*") != 0)) {
> > + /* Invalid request-target per rfc7230#section-5.3 */
> > + status = APR_EINVAL;
> > + }
> > }
> >
> > if (status == APR_SUCCESS) {
>
> Don't we miss in server/protocol.c:
>
> @@ -906,6 +911,12 @@
>
> ap_parse_uri(r, uri);
>
> + if (strict && deferred_error == rrl_none
> + && r->status == HTTP_BAD_REQUEST) {
> + deferred_error = rrl_invaliduri
> + }
Somehow r->status != HTTP_OK is caught below all the deferred_error
cases (which we want to report in priority?), and then jumps to the
rrl_failed label.
Regards;
Yann.
