A few things to fix/discuss

[Sai Pullabhotla <sa...@jmethods.com>]

DELE command should NOT delete a file if it is a directory. Clients
should instead use RMD command.

DELE command should not delete the current working directory. I think
most FTP servers return a an error saying the directory is in use. The
other problem by letting clients do a "DELE ." is the server still
thinks the working directory is still what ever they were before the
directory was deleted. So any subsequent commands like PUT would fail.

LIST command on a non-existent directory should result in an error.
Instead, we get the reply 150 File status okay; about to open data
connection. 226 Closing data connection.

MFMT Command - in the source code for this command, the DateFormat and
its configuration should be moved to static block for performance/to
reduce object creations.

MFMT Command replies with just the code 213. We might want to add a
message at least saying requested action was successful.

MFMT Command CANNOT be used on file names that have multiple words.
The command MFMT 20050201013000 "Set Variable Task.doc" (with or
without quotes) fails even though I've a file with the specified name.
The arguments need to be parsed differently if they are enclosed in
double quotes or treat everything after the first argument as the file
name. So, the code in MFMT.java  -
String[] arguments = argument.split(" "); should be changed to
String[] arguments = arguments.split(" ", 2);

MFMT Command should return a Positive Completion Reply if and only if
we are sure that the date on the file was modified. In order to fix
this, we need to USE the return value from
java.io.File.setLastModified(long). We are not using the returned
boolean from this method and assuming that the new date was set. Code
needs to be changed in a few places to fix this. The issue can be
reproduced by trying to set the date on a file that is in use. I used
a .doc file, had it open in MS Word and ran the MFMT command and got
the 213 reply, but the date on the file was not changed.

Not sure if MKD command should create all the parent directories. I
think most FTP servers try to create the last name specified in the
path. For example, MKD /a/b/c should only succeed if /a/b exists. We
create a and b if they do not exist. Something to think about.

Not sure if TYPE command with no argument should change the type to
ASCII. Instead it should reply back with 2xx reply indicating the
current type in use. Not sure what the RFC says.

Sai Pullabhotla
Phone: (402) 408-5753
Fax: (402) 408-6861
www.jMethods.com

Re: A few things to fix/discuss

[Sai Pullabhotla <sa...@jmethods.com>]

David,

I agree with you on replying a 550 message when the path does not exist.

IBM's AS/400 FTP server returns: 550  Path does not exist: zzz
ftp.netscape.com (anpnymous FTP Server) says: 550 zzz: No such file or
directory
Not sure what the server software is, but it claims that it is 220
spftp/1.0.0000 Server [204.2.225.157]

Regards,

Sai Pullabhotla
Phone: (402) 408-5753
Fax: (402) 408-6861
www.jMethods.com




On Thu, Dec 4, 2008 at 5:58 PM, David Latorre <dv...@gmail.com> wrote:
> Hey ,
>
>  Thanks for your great feedback and excuse me if I incorrectly closed some
> issues. There are things you cannot do at 2 am with a fever.
>
> I hope after my last commit, everything is ok - although there are some
> issues still open, of course. By the way, fixes are in trunk; they arrived
> late for M4.
>
>
> The LIST issue I find it difficult as it seems there's no agreement on the
> return codes when pathname is incorrect: some server will always return a
> 216 code (I saw this in pureFTPd ) with some returning a different string
> when the pathname includes a non-existent directory (I saw this in vsFTPd).
> PROFTPd seems to answer with 450 whenever 'pathname'  cannot be found.
>
>  I myself would say that  "File not found" should be 550 and not 450 (and
> this is not a response code for LIST, according to the RFC) but I'm probably
> totally wrong  ...
>
> What servers/clients do you guys check for this?
>
>
> Cheers ,
>
>  David Latorre
>
>
> 2008/12/4 Niklas Gustavsson <ni...@protocol7.com>
>
>> On Wed, Dec 3, 2008 at 11:20 PM, Sai Pullabhotla
>> <sa...@jmethods.com> wrote:
>> > I've opened Jira issues. Let me know if it is okay with you to fix
>> > some of the easier/obvious ones for M4. I can work on some/all of
>> > these fairly quick.
>>
>> I would not like to include these in M4 but rather in RC1 (want to get
>> M4 out). If you can provide patches that would be great.
>>
>> Again, thanks for your feedback!
>>
>> /niklas
>>
>

Re: A few things to fix/discuss

[David Latorre <dv...@gmail.com>]

Hey ,

 Thanks for your great feedback and excuse me if I incorrectly closed some
issues. There are things you cannot do at 2 am with a fever.

I hope after my last commit, everything is ok - although there are some
issues still open, of course. By the way, fixes are in trunk; they arrived
late for M4.


The LIST issue I find it difficult as it seems there's no agreement on the
return codes when pathname is incorrect: some server will always return a
216 code (I saw this in pureFTPd ) with some returning a different string
when the pathname includes a non-existent directory (I saw this in vsFTPd).
PROFTPd seems to answer with 450 whenever 'pathname'  cannot be found.

 I myself would say that  "File not found" should be 550 and not 450 (and
this is not a response code for LIST, according to the RFC) but I'm probably
totally wrong  ...

What servers/clients do you guys check for this?


Cheers ,

 David Latorre


2008/12/4 Niklas Gustavsson <ni...@protocol7.com>

> On Wed, Dec 3, 2008 at 11:20 PM, Sai Pullabhotla
> <sa...@jmethods.com> wrote:
> > I've opened Jira issues. Let me know if it is okay with you to fix
> > some of the easier/obvious ones for M4. I can work on some/all of
> > these fairly quick.
>
> I would not like to include these in M4 but rather in RC1 (want to get
> M4 out). If you can provide patches that would be great.
>
> Again, thanks for your feedback!
>
> /niklas
>

Re: A few things to fix/discuss

[Niklas Gustavsson <ni...@protocol7.com>]

On Wed, Dec 3, 2008 at 11:20 PM, Sai Pullabhotla
<sa...@jmethods.com> wrote:
> I've opened Jira issues. Let me know if it is okay with you to fix
> some of the easier/obvious ones for M4. I can work on some/all of
> these fairly quick.

I would not like to include these in M4 but rather in RC1 (want to get
M4 out). If you can provide patches that would be great.

Again, thanks for your feedback!

/niklas

Re: A few things to fix/discuss

[Sai Pullabhotla <sa...@jmethods.com>]

Niklas,

I've opened Jira issues. Let me know if it is okay with you to fix
some of the easier/obvious ones for M4. I can work on some/all of
these fairly quick.

Regatds,

Sai Pullabhotla
Phone: (402) 408-5753
Fax: (402) 408-6861
www.jMethods.com




On Wed, Dec 3, 2008 at 1:52 PM, Niklas Gustavsson <ni...@protocol7.com> wrote:
> This is outstanding feedback! Could I ask you for the trouble to
> create these as individual JIRA issues? We will have a look at fixing
> them before RC1.
>
> /niklas
>
> On Wed, Dec 3, 2008 at 8:04 PM, Sai Pullabhotla
> <sa...@jmethods.com> wrote:
>> DELE command should NOT delete a file if it is a directory. Clients
>> should instead use RMD command.
>>
>> DELE command should not delete the current working directory. I think
>> most FTP servers return a an error saying the directory is in use. The
>> other problem by letting clients do a "DELE ." is the server still
>> thinks the working directory is still what ever they were before the
>> directory was deleted. So any subsequent commands like PUT would fail.
>>
>> LIST command on a non-existent directory should result in an error.
>> Instead, we get the reply 150 File status okay; about to open data
>> connection. 226 Closing data connection.
>>
>> MFMT Command - in the source code for this command, the DateFormat and
>> its configuration should be moved to static block for performance/to
>> reduce object creations.
>>
>> MFMT Command replies with just the code 213. We might want to add a
>> message at least saying requested action was successful.
>>
>> MFMT Command CANNOT be used on file names that have multiple words.
>> The command MFMT 20050201013000 "Set Variable Task.doc" (with or
>> without quotes) fails even though I've a file with the specified name.
>> The arguments need to be parsed differently if they are enclosed in
>> double quotes or treat everything after the first argument as the file
>> name. So, the code in MFMT.java  -
>> String[] arguments = argument.split(" "); should be changed to
>> String[] arguments = arguments.split(" ", 2);
>>
>> MFMT Command should return a Positive Completion Reply if and only if
>> we are sure that the date on the file was modified. In order to fix
>> this, we need to USE the return value from
>> java.io.File.setLastModified(long). We are not using the returned
>> boolean from this method and assuming that the new date was set. Code
>> needs to be changed in a few places to fix this. The issue can be
>> reproduced by trying to set the date on a file that is in use. I used
>> a .doc file, had it open in MS Word and ran the MFMT command and got
>> the 213 reply, but the date on the file was not changed.
>>
>> Not sure if MKD command should create all the parent directories. I
>> think most FTP servers try to create the last name specified in the
>> path. For example, MKD /a/b/c should only succeed if /a/b exists. We
>> create a and b if they do not exist. Something to think about.
>>
>> Not sure if TYPE command with no argument should change the type to
>> ASCII. Instead it should reply back with 2xx reply indicating the
>> current type in use. Not sure what the RFC says.
>>
>> Sai Pullabhotla
>> Phone: (402) 408-5753
>> Fax: (402) 408-6861
>> www.jMethods.com
>>
>

Re: A few things to fix/discuss

[Niklas Gustavsson <ni...@protocol7.com>]

This is outstanding feedback! Could I ask you for the trouble to
create these as individual JIRA issues? We will have a look at fixing
them before RC1.

/niklas

On Wed, Dec 3, 2008 at 8:04 PM, Sai Pullabhotla
<sa...@jmethods.com> wrote:
> DELE command should NOT delete a file if it is a directory. Clients
> should instead use RMD command.
>
> DELE command should not delete the current working directory. I think
> most FTP servers return a an error saying the directory is in use. The
> other problem by letting clients do a "DELE ." is the server still
> thinks the working directory is still what ever they were before the
> directory was deleted. So any subsequent commands like PUT would fail.
>
> LIST command on a non-existent directory should result in an error.
> Instead, we get the reply 150 File status okay; about to open data
> connection. 226 Closing data connection.
>
> MFMT Command - in the source code for this command, the DateFormat and
> its configuration should be moved to static block for performance/to
> reduce object creations.
>
> MFMT Command replies with just the code 213. We might want to add a
> message at least saying requested action was successful.
>
> MFMT Command CANNOT be used on file names that have multiple words.
> The command MFMT 20050201013000 "Set Variable Task.doc" (with or
> without quotes) fails even though I've a file with the specified name.
> The arguments need to be parsed differently if they are enclosed in
> double quotes or treat everything after the first argument as the file
> name. So, the code in MFMT.java  -
> String[] arguments = argument.split(" "); should be changed to
> String[] arguments = arguments.split(" ", 2);
>
> MFMT Command should return a Positive Completion Reply if and only if
> we are sure that the date on the file was modified. In order to fix
> this, we need to USE the return value from
> java.io.File.setLastModified(long). We are not using the returned
> boolean from this method and assuming that the new date was set. Code
> needs to be changed in a few places to fix this. The issue can be
> reproduced by trying to set the date on a file that is in use. I used
> a .doc file, had it open in MS Word and ran the MFMT command and got
> the 213 reply, but the date on the file was not changed.
>
> Not sure if MKD command should create all the parent directories. I
> think most FTP servers try to create the last name specified in the
> path. For example, MKD /a/b/c should only succeed if /a/b exists. We
> create a and b if they do not exist. Something to think about.
>
> Not sure if TYPE command with no argument should change the type to
> ASCII. Instead it should reply back with 2xx reply indicating the
> current type in use. Not sure what the RFC says.
>
> Sai Pullabhotla
> Phone: (402) 408-5753
> Fax: (402) 408-6861
> www.jMethods.com
>