robinrodricks / FluentFTP

An FTP and FTPS client for .NET & .NET Standard, optimized for speed. Provides extensive FTP commands, File uploads/downloads, SSL/TLS connections, Automatic directory listing parsing, File hashing/checksums, File permissions/CHMOD, FTP proxies, FXP support, UTF-8 support, Async/await support, Powershell support and more. Written entirely in C#.
MIT License
2.96k stars 643 forks source link

GetObjectInfo fails for file names with spaces #1567

Closed anandgmenon closed 1 week ago

anandgmenon commented 2 weeks ago

FTP Server OS: Windows

FTP Server Type: WS FTP Client Computer OS: Windows

FluentFTP Version: 49.0.2

Framework: .NETCore3.1

When using GetObjectInfo("file name with spaces") we see the error 501 Invalid number of arguments MLST. I see the error is similar to what we saw for MLSD in https://github.com/robinrodricks/FluentFTP/issues/1309 and the workaround there was to use SetWorkingDirectory("folder name with spaces") and then GetListing("", FtpListOption.NoPath);.

However wondering if there's any workaround for GetObjectInfo in this case. I also saw there was a suggestion to use GetListing("\"WebsiteTestFiles/dev/invoices-staging/bvk/20230808_007_City and state of bvk\"")

but we have code writen to handle different types of servers, so that approach might not work for other servers.

Logs :


>         IsStillConnected(10000)
Command:  NOOP (<-Noop)
Status:   Waiting for response to: NOOP (<-IsStillConnected/Noop)
Response: 200 Command NOOP succeed [50ms]
>         GetObjectInfo("/users/daniel_int/YLOCM_R_FTP_GET_YUNAS-OFF-AFF_FF315/Event Extract - Forwarding (2024-03-1117-03-37).CSV", False)
Command:  MLST /users/daniel_int/YLOCM_R_FTP_GET_YUNAS-OFF-AFF_FF315/Event Extract - Forwarding (2024-03-1117-03-37).CSV
Status:   Waiting for response to: MLST /users/daniel_int/YLOCM_R_FTP_GET_YUNAS-OFF-AFF_FF315/Event Extract - Forwarding (2024-03-1117-03-37).CSV
Response: 501 Invalid number of arguments MLST /users/daniel_int/YLOCM_R_FTP_GET_YUNAS-OFF-AFF_FF315/Event Extract - Forwarding (2024-03-1117-03-37).CSV [56ms]
Warning:  Failed to get object info for path /users/daniel_int/YLOCM_R_FTP_GET_YUNAS-OFF-AFF_FF315/Event Extract - Forwarding (2024-03-1117-03-37).CSV with error Invalid number of arguments MLST /users/daniel_int/YLOCM_R_FTP_GET_YUNAS-OFF-AFF_FF315/Event Extract - Forwarding (2024-03-1117-03-37).CSV
FanDjango commented 2 weeks ago

Current implementation of GetObjectInfo will always use MLST. It does not provide for any parameters allowing you to specify any kind of "special" behaviour. GetListing has many such provisions, such as ".NoPath", or ".ForceList", but GetObjectInfo does not.

That being said, one would assume that such options would also be useful for GetObjectInfo.

Note, that on servers that do not support MLST (see the response to the FEAT command at connect time), GetObjectInfo reverts to using GetListing, but still does not utilise special options.

So, unless GetObjectInfo is enhanced to handle recalcitrant servers with some additional options (not planned right not, but maybe this year?), I see only that you should stop using GetObjectInfo and replace it with GetListing and any addional processing you might need by yourself.

Note that the RFCs for FTP state that any command that requires a "file/pathspec" have this be the last parameter of the command and that all characters up to the end of the line are part of the "file/pathspec", even blanks or other characters. So a server that fails because of blanks in a file/pathspec is not working according to the RFCs and thus can be considered to be "broken".

You really need to read: This chapter in the RFCs

So this is not a bug. It is an enhancement request.

Since GetListing can be used by you to ascertain the same information as GetObjectInfo does and since it does allow you to force the use of LIST instead of MLST, thereby avoiding the "filename contains blank issue" (of WSFTPD), I would advise you to choose this path for now. Meanwhile, internally here we will discuss whether allocating meager resources to "bypass" the quirks of inferior server implementations would be advisable.

FanDjango commented 2 weeks ago

I should add that filenames containing blanks in combination with servers whose characteristics you do not control is a bad decision. You could perhaps envision reducing your file-naming, software design and algorithms to a very restricted subset that accomodates the lowest common denominator. Many of the things we take for granted (such as file names with blanks) were very lately added as extensions to FTP compared to the origins.

On the other hand, since WSFTP is (AFAIK) commercial software, have you thought about referring this problem to them? I appreciate that you and your product seem to be accessing these servers as part of your offering itself, but you should perhaps also indicate to your users that there is a problem here in their choice of server and that there is a limit as what can be done to remedy that on the client side.

anandgmenon commented 2 weeks ago

@FanDjango thanks a lot for the detailed explanantion.

Since GetListing can be used by you to ascertain the same information as GetObjectInfo does and since it does allow you to force the use of LIST instead of MLST

Will look into implementing this as a long term fix on our end, only concern right now is we have seen usecases where clients do have large number of files and we really do not want to do LIST everytime instead of MLST (even though I understand we do LIST already for servers which do not support MLST)

You could perhaps envision reducing your file-naming, software design and algorithms to a very restricted subset that accomodates the lowest common denominator.

Considering the range of FTP servers we support, I really doubt if we can really enforce the file names to have a subset of characters, because like in this case whitespaces works fine for some other servers or other commands as well.

On the other hand, since WSFTP is (AFAIK) commercial software, have you thought about referring this problem to them?

Had one question this, is this specific to WSFTP or applies to all servers implementing MLST?

FanDjango commented 2 weeks ago

even though I understand we do LIST already for servers which do not support MLST

we really do not want to do LIST everytime instead of MLST

No, not unconditionally. But how about logic similar to the following;

GetObjectInfo(filepath+name) If failure and (filepath+name contains blanks) ChangeWorkingDirectory...... GetListing....

Meaning, attempt the GOI, and only resort to a GetListing on failure.

On the other hand, if you maintain a list of server-types that misbehave, you could branch directly.

is this specific to WSFTP or applies to all servers implementing MLST?

Oh no, no. Read the RFCs again. It is specific to WSFTP (that version, actually you make no mention of what WSFTP version this might be). I have no idea if updates to WSFTP might even have fixed that non-RFC behaviour. Also some primitive embedded systems might have faulty parameter parsing of the type that this WSFTP server is demonstrating.

I repeat: As per RFCs MLST takes a filepath as parameter. A filepath is all chars up to the end of the line including all blanks, special characters etc. etc. All servers that I myself use and know of, work fine with filenames containing blanks.

FanDjango commented 1 week ago

@anandgmenon Can we close this?

anandgmenon commented 1 week ago

sure, thanks for your detailed explanantion on the issue :)