Notice using FILEUPLOAD (in debug mode)

[lonny27]

Hi,

If run the script with define('DEBUG',true); and encountered following behavior. It's not an error, but looks like a bug which php is covering.

Version 8.05
Line numbers refer frommail.php from the download formmail.zip

In line 6814 in function function SubstituteValueWorker the following statement

if (is_array($aSubstituteValues[$s_name]))

leads to the notice

Notice: Undefined index: upload_file in formail.php on line 6814

if upload_file is the name of an <input type="file" name="upload_file" ...>

Reason: in line 6811 and 6812 two checks were performed if the variable exists and is not empty.

if (IsFieldSet($s_name,$aSubstituteValues) &&
!TestFieldEmpty($s_name,$aSubstituteValues,$s_mesg))

Both function do not check the array given in the function parameter (here $aSubstituteValues) if FILEUPLOAD is set.

function IsFieldSet($s_fld,$a_main_vars)
{
global $aFileVars;

return (isset($a_main_vars[$s_fld]) ||
(FILEUPLOADS && isset($aFileVars[$s_fld])));
}

Don't know if it's wroth a fix, because it doesn't lead to an error... but the two functions are really bad software design, which leads to this issue.

Regards,
Lonny

[russellr]

Hi,

In line 6814 in function function SubstituteValueWorker the following statement

if (is_array($aSubstituteValues[$s_name]))

Yes. The code should be:

        if (isset($aSubstituteValues[$s_name]) &&
            is_array($aSubstituteValues[$s_name])) 

if (IsFieldSet($s_name,$aSubstituteValues) &&
!TestFieldEmpty($s_name,$aSubstituteValues,$s_mesg))

Both function do not check the array given in the function parameter (here $aSubstituteValues) if FILEUPLOAD is set.

Not quite true. Both functions do check the array in the function parameter ($aSubstituteValues) regardless of the setting of FILEUPLOAD.

The problem is that IsFieldSet and TestFieldEmpty try to cope with "fields" of any type, including file upload fields.

Originally, they didn't and only dealt with non-file fields. Software evolution often causes little problems like this.

Don't know if it's wroth a fix, because it doesn't lead to an error... but the two functions are really bad software design, which leads to this issue.

Unfortunately, PHP provides some fields in a different array. File upload fields are in a different array to all other fields, and this makes coding quite difficult. Plus, you add in the option of disabling file uploads.

Anyway, easily fixed.

Unfortunately, I've never seen these error messages out of PHP, otherwise the code would have been corrected earlier. Perhaps a new version of PHP is behaving differently.

[lonny27]

Not quite true. Both functions do check the array in the function parameter ($aSubstituteValues) regardless of the setting of FILEUPLOAD.

The problem is that IsFieldSet and TestFieldEmpty try to cope with "fields" of any type, including file upload fields.

Originally, they didn't and only dealt with non-file fields. Software evolution often causes little problems like this.

Yes, the array gets checked. What I wanted to say was, that the result of the function IsFieldSet($s_fld,$a_main_vars) is if
true: yes, $s_fld is set in $a_main_vars
false: maybe, $s_fld is set in $a_main_vars

But I understand that software evolution leads sometimes to strange constructs
With more than 10.000 lines the script is quite complex, and due to the fact that it's procedural and not object oriented makes it even more difficult to extend.

Anyway it's a great script and it's one of the best documented open source projects I've ever seen!

Best regards,
Lonny

[russellr]

Hi,

Yes, the array gets checked. What I wanted to say was, that the result of the

function IsFieldSet($s_fld,$a_main_vars) is if
true: yes, $s_fld is set in $a_main_vars
false: maybe, $s_fld is set in $a_main_vars

I'm not sure what you're saying above.

IsFieldSet always returns true if $s_fld is set in $a_main_vars.

If $s_fld is set in $aFileVars, then the function returns true if you've configured to enable file uploads.

The logic is exactly right for the purpose of the function.

With more than 10.000 lines the script is quite complex, and due to the fact that it's procedural and not object oriented makes it even more difficult to extend.

The next major version is being re-written as object-oriented. This will allow us to easily exclude code people don't want, thereby reducing the size of the script for people who only want basic functionality.

Anyway it's a great script and it's one of the best documented open source projects I've ever seen!

Thanks.