[FFmpeg-devel] [PATCH 3/3] lavf/ftp: implement NLST method

[Michael Niedermayer]

On Tue, Aug 11, 2015 at 12:42:04AM +0200, Mariusz Szczepańczyk wrote:
> On Mon, Aug 10, 2015 at 6:41 PM, Michael Niedermayer <michael at niedermayer.cc
> > wrote:
> 
> > On Sun, Aug 09, 2015 at 02:38:12AM +0200, Mariusz Szczepańczyk wrote:
> > > ---
> > >  libavformat/ftp.c | 98
> > +++++++++++++++++++++++++++++++++++++++++++++++--------
> > >  1 file changed, 84 insertions(+), 14 deletions(-)
> > >
> > > diff --git a/libavformat/ftp.c b/libavformat/ftp.c
> > > index 542cf6a..5a312b9 100644
> > > --- a/libavformat/ftp.c
> > > +++ b/libavformat/ftp.c
> > > @@ -38,6 +38,12 @@ typedef enum {
> > >      DISCONNECTED
> > >  } FTPState;
> > >
> > > +typedef enum {
> > > +    UNKNOWN_METHOD,
> > > +    NLST,
> > > +    MLSD
> > > +} FTPListingMethod;
> > > +
> > >  typedef struct {
> > >      const AVClass *class;
> > >      URLContext *conn_control;                    /**< Control
> > connection */
> > > @@ -56,6 +62,8 @@ typedef struct {
> > >      const char *anonymous_password;              /**< Password to be
> > used for anonymous user. An email should be used. */
> > >      int write_seekable;                          /**< Control
> > seekability, 0 = disable, 1 = enable. */
> > >      FTPState state;                              /**< State of data
> > connection */
> > > +    FTPListingMethod listing_method;             /**< Called listing
> > method */
> > > +    char *features;                              /**< List of server's
> > features represented as raw response */
> > >      char *dir_buffer;
> > >      size_t dir_buffer_size;
> > >      size_t dir_buffer_offset;
> > > @@ -192,6 +200,8 @@ static int ftp_send_command(FTPContext *s, const
> > char *command,
> > >  {
> > >      int err;
> > >
> > > +    av_dlog(s, "%s", command);
> > > +
> > >      if (response)
> > >          *response = NULL;
> > >
> > > @@ -449,32 +459,68 @@ static int ftp_set_dir(FTPContext *s)
> > >      return 0;
> > >  }
> > >
> > > -static int ftp_list(FTPContext *s)
> > > +static int ftp_list_mlsd(FTPContext *s)
> > >  {
> > >      static const char *command = "MLSD\r\n";
> > >      static const int mlsd_codes[] = {150, 500, 0}; /* 500 is incorrect
> > code */
> > >
> > >      if (ftp_send_command(s, command, mlsd_codes, NULL) != 150)
> > >          return AVERROR(ENOSYS);
> > > -    s->state = LISTING_DIR;
> > > +    s->listing_method = MLSD;
> > > +    return 0;
> > > +}
> > > +
> > > +static int ftp_list_nlst(FTPContext *s)
> > > +{
> > > +    static const char *command = "NLST\r\n";
> > > +    static const int nlst_codes[] = {226, 425, 426, 451, 450, 550, 0};
> > > +
> > > +    if (ftp_send_command(s, command, nlst_codes, NULL) != 226)
> > > +        return AVERROR(ENOSYS);
> > > +    s->listing_method = NLST;
> > >      return 0;
> > >  }
> > >
> > > +static int ftp_has_feature(FTPContext *s, const char *feature_name);
> > > +
> > > +static int ftp_list(FTPContext *s)
> > > +{
> > > +    s->state = LISTING_DIR;
> > > +
> > > +    if (ftp_has_feature(s, "MLSD"))
> > > +        return ftp_list_mlsd(s);
> > > +
> > > +    return ftp_list_nlst(s);
> > > +}
> > > +
> > > +static int ftp_has_feature(FTPContext *s, const char *feature_name)
> > > +{
> > > +    if (!s->features)
> > > +        return 0;
> > > +
> > > +    return av_stristr(s->features, feature_name) != NULL;
> > > +}
> > > +
> > >  static int ftp_features(FTPContext *s)
> > >  {
> > >      static const char *feat_command        = "FEAT\r\n";
> > >      static const char *enable_utf8_command = "OPTS UTF8 ON\r\n";
> > >      static const int feat_codes[] = {211, 0};
> > >      static const int opts_codes[] = {200, 451, 0};
> > > -    char *feat = NULL;
> >
> > > +    if (s->features) {
> > > +        av_freep(&s->features);
> > > +        s->features = NULL;
> > > +    }
> >
> > the if and NULL setting is uneeded
> >
> >
> fixed
> 
> 
> > also please explain how each patch can be tested
> >
> >
> For now I'm using local ftp servers, and netcat to test "wrong" responses.
> I'd like to write some blackbox tests (list of input commands + expected
> responses) but I'm still thinking how to fit them into the current
> architecture.
[...]
> > [...]
> > --
> > Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
> >
> > Old school: Use the lowest level language in which you can solve the
> > problem
> >             conveniently.
> > New school: Use the highest level language in which the latest
> > supercomputer
> >             can solve the problem without the user falling asleep waiting.
> >
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel at ffmpeg.org
> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
> >

>  ftp.c |   94 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++----------
>  1 file changed, 80 insertions(+), 14 deletions(-)
> ae23918e29360a8a96c9884ed27d3aac4844e7bc  0003-lavf-ftp-implement-NLST-method.patch
> From f682cc6da7fc21f8ae5d646ccc3ffd7fb6c3a993 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Mariusz=20Szczepa=C5=84czyk?= <mszczepanczyk at gmail.com>
> Date: Tue, 16 Jun 2015 20:41:31 +0200
> Subject: [PATCH 3/3] lavf/ftp: implement NLST method

this patch breaks
doc/examples/avio_list_dir ftp://ftp.idsoftware.com

please test your code with some random servers out in the wild

also fate tests or other would be highly welcome, but yes as you
say its not clear how to fit that in

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I am the wisest man alive, for I know one thing, and that is that I know
nothing. -- Socrates
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20150811/01d0e4db/attachment.sig>