Issue in FOGFTP.class.php and in Host.class.php
-
I detected several issues in those two files, mainly with error handling.
SVN 3508
- 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.
-
@glefebvr
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.
-
3516 Should, attempt, address the username issue.
-
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
-
Dear Tom,
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.
-
@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?
-
@Tom-Elliott
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 -
Well I added the stripslashes to the FOGFTP and it is committed.
I’m going to solve this thread unless you see another issue.
