URI parameter is declared As String instead of System.Uri

ID

vbnet.maintainability.uri_parameters_not_strings

Severity

low

Remediation Complexity

medium

Remediation Risk

medium

Remediation Effort

medium

Resource

Api Design

Language

VB.NET

Description

Reports a parameter declared As String whose name indicates that it carries a URI - the name is exactly uri, url or urn, or it ends in Uri, Url, Urn, URI, URL or URN, as in serviceUrl, baseUri or callbackURL. Parameters of Public methods, functions, constructors and shared members are examined, as are members declared on an interface; ByVal, ByRef and Optional parameters are all covered. Since the check keys on the parameter name, a URI parameter named without one of those words is not reported, and a parameter already typed Uri is accepted whatever it is called.

Rationale

A URI passed as a String is an unvalidated URI. Nothing at the call site or inside the method establishes that the value is well formed, absolute, or of a scheme the method can act on, so the check has to happen somewhere - and because the signature does not demand it, it usually happens nowhere, or happens differently in each overload and each caller. The failure then surfaces deep inside whatever consumes the value, as a UriFormatException from a constructor several frames away or, worse, as a silently wrong request when the string is concatenated into a larger URI. That concatenation is the second problem: string-typed URIs invite manual assembly with & and String.Format, which is exactly where escaping, double slashes, relative-versus-absolute confusion and query-string encoding bugs come from, whereas Uri exposes the components and does the escaping itself. String also loses the comparison semantics that URIs need - Uri.Equals normalizes case in the scheme and host and ignores the fragment, while String.Equals treats HTTP://Host/a and http://host/a as different values, so caches, dictionaries and equality checks keyed on the string form quietly miss. Changing the parameter type later is a breaking change for every caller, so the cost of leaving it grows with the number of consumers.

The following code illustrates the pattern detected by this rule:

Public Property Endpoint As Uri

' FLAGGED: URI parameter is declared As String instead of System.Uri
Public Sub Fetch(url As String)
    Dim client As New HttpClient()
    client.GetStringAsync(url)
End Sub

Remediation

Declare the parameter As Uri and let the caller construct it, so the value is validated once, at the point where the URI text is known, and the method receives something already parsed. Build derived URIs with New Uri(baseUri, relativePath) or UriBuilder rather than concatenating strings, and use Uri.EscapeDataString for query values. For a public API that must stay convenient, keep a String overload alongside the Uri one - have it do nothing but New Uri(…​) and delegate - rather than replacing the typed parameter; for an existing API that cannot break callers, add the Uri overload and mark the String one <Obsolete>. If the parameter really accepts a relative fragment rather than a URI, rename it so the name no longer claims otherwise.

Configuration

This detector does not need any configuration.

References