Bug 85257

Comment 1 Sergey Vladimirov 2005-02-15 09:25:41 EST

Versions of Eclipse and FTP&WebDAV - 3.1M4

Comment 2 Michael Valenta 2005-02-15 09:47:27 EST

The FTP plugin is not being actively developed. Patches will be accepted.

Comment 3 Sergey Vladimirov 2005-02-16 09:10:34 EST

I have no censured words. Just only this fix:

package org.eclipse.ftp.internal;
public class FTPClient implements IClient {
<...>
	public IDirectoryEntry[] listFiles(String directoryPath, boolean 
includeParents, IProgressMonitor monitor) throws FtpException {
		monitor = FtpPlugin.monitorFor(monitor);
		monitor.beginTask(null, 115);
		final String title = FtpPlugin.getResourceString
("FTPClient.listFiles", directoryPath == null ? "." : directoryPath); //$NON-
NLS-1$ //$NON-NLS-2$
		monitor.subTask(title);
		DataConnection manager = null;
		try {
            
            if (directoryPath != null) {
                sendCommand("CWD", directoryPath); //$NON-NLS-1$
                monitor.worked(5);
                switch (readResponseSkip100()) {
                    case 250: 
                        // requested file action ok
                        break;
                    default:
                        defaultResponseHandler();
                }
                monitor.worked(10);
            } else {
                monitor.worked(15);
            }

			// Initiate the data transfer
			manager = initiateDataTransfer("LIST -a", null, false, 
0, FtpPlugin.monitorFor(monitor)); //$NON-NLS-1$
			// Get the input stream from the data connection
			monitor.subTask(FtpPlugin.getResourceString
("FTPClient.transferNoSize", title)); //$NON-NLS-1$

<...>

Comment 4 Sergey Vladimirov 2005-02-16 10:24:52 EST

another fix for another issue.

package org.eclipse.ftp.internal;
<...>
public class FTPClient implements IClient {
<...>
	private InputStream internalGetContents(String filePath, boolean 
binary, long resumeAt, IProgressMonitor monitor) throws FtpException {
		Assert.isNotNull(filePath);

		DataConnection manager = null;
		// send the request to the server
		int tries = 0;
		FtpException lastExc = null;
		while (tries < 3) {
			try {
				manager = initiateDataTransfer(
						"RETR", filePath, binary, 
resumeAt, FtpPlugin.monitorFor(monitor)); //$NON-NLS-1$
				lastExc = null;
			} catch (FtpException exc) {
				lastExc = exc;
			}
			tries++;
			if (lastExc == null) {
				break;
			}
		}
		if (lastExc != null) {
			throw lastExc; 
		}
		InputStream in = manager.getDataTransferInputStream();

		// setup line terminator conversion
		if (! binary && ! Constants.IS_CRLF_PLATFORM) in = new 
CRLFtoLFInputStream(in);
		return in;
	}
<...>

Comment 5 Michael Valenta 2005-02-22 15:03:52 EST

The code in comment #3 breaks the client for me (i.e. I can no longer expand 
my FTP site in the site explorer. I'm not keen on the code in comment #4 
either since all it seems to do is try a few more times.

Comment 6 Sergey Vladimirov 2005-02-25 06:32:41 EST

Can you provide FTP account to test and fix changes?

Comment 7 Michael Valenta 2005-02-25 08:44:58 EST

Sorry, I do not have the ability to do that. However, the ftp server I ran 
against was the obe that ships with RedHat 9.0.

Comment 8 Guillaume Boudreau 2005-02-25 16:48:36 EST

Created attachment 18326 [details]
Patch for org/eclipse/ftp/internal/FTPClient.java v1.2

Comment 9 Guillaume Boudreau 2005-02-25 16:56:49 EST

There is a (probably related) bug that makes the FTP plugin list all 1st level
subdirs of the directory when listing it.
ie opening "home" will make the plugin send all those commands:

LIST -a home/
LIST -a home/backup
LIST -a home/gboudreau
etc.

which takes quite some time to execute...

Comment 10 Guillaume Boudreau 2005-02-25 18:02:53 EST

Created attachment 18327 [details]
Patch for org/eclipse/target/internal/ftp/FtpSite.java v1.2 and org/eclipse/target/internal/ftp/FtpTargetResource.java v1.3

Problem was deeper than I thought...
The TreeView is calling hasChildren() to check if it will display a "+" next to
the item; thus the (mostly) unneeded LIST -a called succesively.
The attached patch 'fix' this by keeping a counter of how many directories
there are under the current directory and skipping the next calls to LIST that
many times after that. Each time a LIST is skipped, a dummy element is returned
so that the TreeView will display a "+" for the item.

For me, the FtpPlugin was unusable without this patch... Listing my /home/pub/
was taking about a minute (~ 40 subdirs)...

Comment 11 Michael Valenta 2005-04-18 14:37:17 EDT

I have submited the fix from comment 8. The fix in comment 10 made some 
assumptions that I didn't feel were safe to release but I found an alternate 
work around for that issue.