LDEV-5030 - Bugfix Setup SMTP-Connection

[Xcreen]

Bug-Fix: Allow Setup/Edit/Verify/Delete SMTP Server while "Limit variable evaluation" is enabled

https://luceeserver.atlassian.net/jira/software/c/projects/LDEV/issues/LDEV-5030

[zspitzer]

@Xcreen thanks for the PR!

This is very old code, simply switching to arrayIsDefined, is just patching around the underlying problem, which is that the utility function toArrayFromForm can leave nulls in the array

A better solution would be to simply ensure that these arrays never contain null values, that way all this old style isDefined, param, etc type logic can be removed altogether

function toArrayFromForm(fieldName) {
	var len=len(fieldName)+1;
	var rtn=array();
	for(var key in form) {
		if(findNoCase(fieldName&"_",key) EQ 1) {
			var index=right(key,len(key)-len);
			if(isNumeric(index))rtn[index]=form[key];
		}
	}
	for (index = 1; index < len(rtn); index++){
	    if (isNull(rtn[index])) rtn[index]="";
	}
	return rtn;
}

https://trycf.com/gist/880650312cc365feccba4db2cfb59dcc/lucee5?theme=monokai

Can you update the PR to use this approach, I will refactor all the admin code for 6.2 to use this approach and remove all the isDefined workarounds completely

[Xcreen]

Hi @zspitzer thank you for the fast feedback.
I can rewrite it with the approach you defined. I already thought that other code also needs rewrite, but i was unsure how to test it, so i was only fixing that script.

Just a question due the "Limited Variable Evaluation":
When it´s enabled the script throws the error due <cfparam name="data.ports[idx]" default="25">.
But its not throwing a error on <cfif isDefined("data.rows[#idx#]"), but for my understanding it should also throw an error?

[zspitzer]

Yeah I was curious about that as well, could be changing the #idx# to just idx might make a difference but I don't know.

PS I know this is crufty old code, split any formatting changes whitespace, into different commits, makes reviews easier

[Xcreen]

After testing a bit around with our change in the toArrayFromForm-function.
I dont think its working without a major rewrite.
It´s not only changing the isDefined to a string-length check. Also the change make arrays larger. E.g a array with 2 entries where one value is null currently leads to a array with a size of one. With the change its keeps the index with a empty value, so the array length is two.
That causes problems like in the services.mail.cfm line 49

loop array=days index="local.i" item="local.day" {
   rtn[i]=createTimeSpan(day,hours[i],minutes[i],seconds[i]);
}

The iteration than runs twice and the createTimeSpan-function throws error because of empty strings.
So every admin-script needs to be tested and migrated.
I guess thats out of the scope for this issue.

I would leave this PR at is it or i could create a second function toArrayFromFormWithEmptyData and migrate the services.mail.cfm to it.