Issue in FOGFTP.class.php and in Host.class.php
I detected several issues in those two files, mainly with error handling.
- Issue 1 : FTP username containg backslashes are not supportet. That’s annoying when using a ftp user from Active Directory
- Issue 2 : file FOGFTP.class.php, function connect : it currently returns either an objet ($this)or an array ($error). IMO it should return either an objet ($this)or a boolean (false)
- Issue 3 : file Host.class.php, function checkIfExist : if the FTP connection goes wrong, the function returns true. ie : it can’t connect to the FTP, so it can’t check if the dir exists, but the function still returns true. Odd !
Thank you in advance for checking this.
Well I added the stripslashes to the FOGFTP and it is committed.
I’m going to solve this thread unless you see another issue.
With stripslashes, the username looks like this : ad\ftpfog
With nothing, the username looks like this : ad\ftpfog
With addslashes, the username looks like this : ad\\ftpfog
@glefebvr So rather than add slashes you are using stripslashes?
Wouldn’t that pose more of an issue?
If your username is \someDomain\alfred, wouldn’t strip slashes make it become someDomainalfred?
Sorry for the delay between my posts. I don’t have much time to test FOG.
Concerning issue 1, I think (and tried) that addslashes is not the solution : now I have 4 backslahes in front of my username. I tried to replace addslashes with stripslashes and this resolves my problem.
Thank you very much for the quick answer and quick resolution of issue n° 1
To explain with more details why I adressed those issues (I should have done that before) :
- I uploaded an image to FOG
- when I wanted to download it on an another computer I receveid the following message : “Failed to create deployment tasking for the following hosts You must first upload an image to create a download task”
- So I decided to check the code and noticed that the FTP connection error was never treated, or at least that it was never presented to me.
- The function “FOGFTP->connect()” always returns a value that is considered as true : either a non empty string (not an array as I was saying before) or an object
- Then, the function $this->FOGFTP->chdir() is called and it has no error handling, so the error is silently dropped
- Finally, the function $this->FOGFTP->close() is called and it has no error handling, so the error is silently dropped
That’s how I noticed that a user containing a backslash was problematic
3516 Should, attempt, address the username issue.
Issue 1: By the use of an addslashes function on the username this may be simple to correct, but it’s just speculation.
Issue 2: This is to be expected. The $error isn’t a boolean result. The idea of passing the error back is so we can print the error information to the screen to help ease/assist in the debugging/issue tracking. If we only return a false statement, troubleshooting what the problem is becomes FAR more difficult and nothing more than a guessing game.
Issue 3: I’ll take a look, but the “return” is not always true, as if any of the ftp functions fail, it will throw it’s own error, thus resulting in that function not returning true.