URI is returned or exposed As String instead of System.Uri

ID

vbnet.maintainability.uri_return_string

Severity

low

Remediation Complexity

medium

Remediation Risk

medium

Remediation Effort

medium

Resource

Api Design

Language

VB.NET

Description

Reports a Public member that hands a URI back to callers as a String rather than a System.Uri: a function whose return type is String, or a property or field declared As String, whose name indicates a URI - the name is exactly uri, url or urn, or it ends in Uri, Url, Urn, URI, URL or URN, as in GetImageUrl, BuildUri, ServiceUrl or MetricsURL. Auto-properties, ReadOnly properties and full Get/Set properties are all covered, as are members declared on an interface. A member already typed Uri is accepted whatever it is called, a non-public member is not examined, and a constant or an initialized field such as Public Const DefaultUrl As String = "…​" is deliberately not reported.

Rationale

A URI returned as text pushes work onto every caller, and each one does it differently. To act on the value a caller must parse it, and because the signature promised only a String, some callers will not - they will concatenate a path onto it with &, compare it to another URI with String.Equals, or pass it straight to something that will fail later. The concatenation cases are where the bugs are: a returned string may or may not end in a slash, may be relative or absolute, and may already carry a query, and manual assembly gets all three wrong sooner or later, while New Uri(base, relative) and UriBuilder handle them by construction. Comparison is the other: Uri.Equals normalizes the scheme and host case and ignores the fragment, so two URIs that address the same resource compare equal, whereas the string forms do not - which quietly breaks caches, dictionaries and de-duplication keyed on the returned value. Returning String also means the object never validated what it is publishing, so a malformed or empty URI escapes the type that owns it and fails somewhere further out, with a stack trace that points at the consumer instead of the source. Because the return type is part of the contract, correcting it later breaks every caller, so the cost rises with the number of consumers.

The following code illustrates the pattern detected by this rule:

Private rawMetrics As String

' FLAGGED: URI is returned or exposed As String instead of System.Uri
Public Property ServiceUrl As String

' FLAGGED: URI is returned or exposed As String instead of System.Uri
Public RawUrl As String

' FLAGGED: URI is returned or exposed As String instead of System.Uri
Public Function GetImageUrl() As String
    Return Me.ServiceUrl
End Function

Remediation

Declare the member As Uri and construct the value inside the type that owns it, so it is validated once, at the point where the URI text is known, and callers receive something already parsed. Build derived URIs with New Uri(baseUri, relativePath) or UriBuilder instead of concatenating the string form, and let callers use Uri.AbsoluteUri or ToString() when they genuinely need text - for display or logging, for instance. For an existing public API that cannot break callers, add the Uri-typed member alongside the String one and mark the old member <Obsolete>; do not keep both as the supported contract. If the member returns a relative fragment or a template rather than a URI, rename it so the name no longer claims otherwise. A public field exposing a URI should become a property in the same change.

Configuration

This detector does not need any configuration.

References