[PATCH 3/3] uclient-fetch: Support for WebDAV methods

Sergey Ponomarev stokito at gmail.com
Sat Apr 8 13:36:20 PDT 2023


Thank you Ellitott for the review,

I agree with all your points. I just made those changes for myself to
support WebDAV methods but generally speaking this is not an important
feature for anyone else.
So let's reject the patch.
The main problem is that any extended http method needs to be
pre-declared inside of the uclient and ideally this should be changed
to be more flexible.
But this may break backward compatibility.

On Sat, Apr 8, 2023 at 1:00 AM Elliott Mitchell <ehem+openwrt at m5p.com> wrote:
>
> On Fri, Apr 07, 2023 at 12:39:09AM +0300, Sergey Ponomarev wrote:
> > The WebDAV is an extension for HTTP for shared folders.
> > In order to make wget working with it we have to declare the missing constants with methods.
> > They don't take part in a logic except of OPTIONS methods that can't have a body.
> >
> > Signed-off-by: Sergey Ponomarev <stokito at gmail.com>
>
> Please examine:
> https://openwrt.org/submitting-patches#submission_guidelines
>
> The commit message needs line-wrapping.
>
> > @@ -292,7 +308,7 @@ static void uclient_http_process_headers(struct uclient_http *uh)
> >
> >  static bool uclient_request_supports_body(enum request_type req_type)
> >  {
> > -     return !(req_type == REQ_GET || req_type == REQ_HEAD);
> > +     return !(req_type == REQ_GET || req_type == REQ_HEAD || req_type == REQ_OPTIONS);
> >  }
> >
> >  static int
>
> Here we have the problem mentioned in the previous message.  That line
> is going to keep growing, and growing, and growing, and growing.  Whereas
> if you merely invert the cases on the switch:
>
> case REQ_GET:
> case REQ_HEAD:
> case REQ_OPTIONS:
>         return false;
>
> default:
>         return true;
>
> The rest seems reasonable.  Again, note I am *not* a maintainer, so this
> is *my* opinion and I do not have any authority (besides experience).
>
>
> --
> (\___(\___(\______          --=> 8-) EHM <=--          ______/)___/)___/)
>  \BS (    |         ehem+sigmsg at m5p.com  PGP 87145445         |    )   /
>   \_CS\   |  _____  -O #include <stddisclaimer.h> O-   _____  |   /  _/
> 8A19\___\_|_/58D2 7E3D DDF4 7BA6 <-PGP-> 41D1 B375 37D0 8714\_|_/___/5445
>
>



More information about the openwrt-devel mailing list