Re: [Bug-wget] wget doesn't handle 5-digit port numbers in EPSV reponses

[Tim Rühsen]

Am Sonntag, 4. Januar 2015, 18:57:23 schrieb Tim Rühsen:
> Am Sonntag, 4. Januar 2015, 16:19:01 schrieb Adam Sampson:
> > Dear wget authors,
> > 
> > When using wget --passive-ftp against IPv6 FTP servers, I occasionally
> > 
> > get the following error:
> >   ==> EPSV ....
> >   Cannot parse PASV response.
> > 
> > I finally found an FTP server that consistently had this problem today
> > 
> > (stunnel.mirt.net), and strace showed that the response in question was:
> >   229 Entering Extended Passive Mode (|||49854|).
> > 
> > This is a perfectly valid response. wget is getting confused because of
> > an off-by-one error in the code that parses the port number in ftp_epsv.
> > When the port number is 5 digits long, i will be 5 at the end of the
> > loop, so the test for an invalid port number length should check for it
> > being *greater than* 5.
> > 
> > Here's the trivial fix:
> > 
> > * ftp-basic.c (ftp_epsv): Accept 5-digit port numbers in EPSV responses.
> > 
> > diff -x config.log -x config.status -ru tmp/wget-1.16.1/src/ftp-basic.c
> > work/wget-1.16.1/src/ftp-basic.c ---
> > tmp/wget-1.16.1/src/ftp-basic.c     2014-12-02 07:49:37.000000000 +0000 +++
> > work/wget-1.16.1/src/ftp-basic.c    2015-01-04 16:06:02.281000000 +0000 
@@
> > -788,7 +788,7 @@
> > 
> >    for (tport = 0, i = 0; i < 5 && c_isdigit (*s); i++, s++)
> >    
> >        tport = (*s - '0') + 10 * tport;
> > 
> > -  if (i >= 5)
> > +  if (i > 5)
> > 
> >      {
> >      
> >        xfree (respline);
> >        return FTPINVPASV;
> > 
> > Thanks very much (and happy new year!),
> 
> Happy New Year, Adam.
> 
> The loop condition is i < 5, so when i becomes 5 the loop stops.
> So how can i be > 5 here ?

Hehe, I was a bit too fast ;-)

i == 5 *is* a valid value after the loop, so you are right.

But since i never becomes > 5, the checks does not make sense and we should 
remove that check. Or change it to what is was meant to be (e.g. i==5 && 
c_isdigit(*s)), I guess.

Tim

signature.asc
Description: This is a digitally signed message part.