Dnn.platform: UpdateModuleSetting SP updates SettingValue case insensitively

[DanielNelsonCantarus]

Description of bug

When only the case of a character in a SettingValue (SV) in the ModuleSettings table is changed, it will not properly update. This is because the check to see if the SV has updated is case insensitive.

The UpdateModuleSetting Stored Procedure compares the old SV with the new in the following section:

WHEN MATCHED AND IsNull(S.SettingValue, N'') != IsNull(Q.SV, N'') THEN -- update only, if value has been modified:
UPDATE SET [SettingValue] = Q.SV, [LastModifiedByUserID] = @UserID, [LastModifiedOnDate] = GetDate()

Due to the collation set on the SettingValue column in the ModuleSettings table, which is not Case Sensitive, the SV will not update if the new value is the same as the old but with the case of a character changed.

Steps to reproduce

List the precise steps to reproduce the bug:

1. Go to a module that has a Module Setting with a SettingValue of the type string
2. Change the case of a character in the string, not adding or removing any other characters.
3. Save the changed Module Setting
4. You can see that the setting has not updated to it's new case.

Current behavior

Module Setting Value does not update if only the case of a character has changed.

Expected behavior

Any change to a Module Setting's value should be properly updated.

Additional context

Two possible solutions that come to mind for this are:

• Change Collation type of SettingValue in ModuleSettings table to make it case sensitive
• Change the Stored Procedure UpdateModuleSetting to set collation type Latin1_General_CS_AS during the comparison between old and new versions of the SettingValue.

Affected version

• [ ] 10.0.0 alpha build
• [ ] 9.5.0 alpha build
• [X] 9.4.4 latest supported release

Affected browser

• [X] Any

[sleupold]

I am strictly against the first solution, as changing collation order on a column might affect sorting as well and cause side effects.
I'll check out the second option - which might need to be applied for update of other settings as well.

[mitchelsellers]

I would tend to agree with Sebastian on this, I think if any change it would need to be temporary for the SP.

But, I also question the need for it to be conditional at this level given that we have .NET Code that also makes it conditional.

[valadas]

I agree, first solution would be problematic for other areas. I would go with either the second option or handle that in code too.

[Timo-Breumelhof]

I would tend to agree with Sebastian on this, I think if any change it would need to be temporary for the SP.

But, I also question the need for it to be conditional at this level given that we have .NET Code that also makes it conditional.

Right, why not just remove the check?

@sleupold , @valadas ?

[sleupold]

I'll have a look and plan to provide a PR