String normalized to lowercase before a comparison instead of to uppercase

ID

vbnet.portability.normalize_strings

Severity

high

Remediation Complexity

trivial

Remediation Risk

low

Remediation Effort

low

Resource

Globalization

Language

VB.NET

Description

Reports ToLower or ToLowerInvariant used to normalise a string that is then compared: either side of an = or <> test, the receiver or argument of Equals, the receiver of StartsWith, EndsWith or Contains, and the expression of a Select Case. Both call forms VB accepts are covered, with and without the empty parentheses - name.ToLower() = "x" and name.ToLower = "x". Lower-casing that feeds something other than a comparison - building a slug, a display label, a URL fragment - is not reported, because there the case of the result is the point.

Rationale

Lower-casing is not a safe round trip for every script, so it is the wrong direction for normalisation. String.ToUpperInvariant is the only casing operation documented as guaranteeing that all inputs which should compare equal end up identical; ToLowerInvariant carries no such guarantee, because for several characters the mapping to lowercase loses the distinction that the uppercase mapping preserves. The classic case is the Turkish dotless ı and dotted i, which collapse differently depending on direction, so two strings a user considers the same can normalise to different lowercase values and the comparison fails - only on those locales, and only for those inputs. The culture-sensitive ToLower() overload adds a second problem on top: it reads the current thread culture, so the same build behaves differently per machine. Neither issue reproduces on a developer workstation configured in English, which is why these defects reach production and then present as an authorisation check or a lookup that inexplicably fails for one group of users. Normalising for comparison also allocates two throwaway strings on every call, which a comparison-aware API avoids.

The following code illustrates the pattern detected by this rule:

Public Function IsAdministrator(ByVal roleName As String) As Boolean
    ' FLAGGED: String normalized to lowercase before a comparison instead of to uppercase
    If roleName.ToLower() = "administrator" Then
        Return True
    End If
    Return False
End Function

Remediation

The best fix is to stop normalising and let the comparison do the work: pass a StringComparison to Equals, StartsWith, EndsWith or String.Compare - StringComparison.OrdinalIgnoreCase for identifiers, keys and other non-linguistic values. That is correct in every culture and allocates nothing. Where an explicitly normalised value really is needed - as a dictionary key, or to store alongside the original - use ToUpperInvariant(), which is the direction documented as safe, and never the culture-sensitive ToLower().

' Before: lower-casing is not a safe round trip, and ToLower() follows the machine's locale
If roleName.ToLower() = "administrator" Then Return True
Return accountName.ToLower().StartsWith("svc-")

' After: let the comparison apply the rules
If String.Equals(roleName, "administrator", StringComparison.OrdinalIgnoreCase) Then Return True
Return accountName.StartsWith("svc-", StringComparison.OrdinalIgnoreCase)

' Or, when a normalised value must be stored
Dim key As String = roleName.ToUpperInvariant()

Configuration

This detector does not need any configuration.

References