Skip to content

Commit

Permalink
Fix StringSegment.Buffer can be null (#57858)
Browse files Browse the repository at this point in the history 
* StringSegment.Buffer can be null

* Throw on null

* Update src/libraries/Microsoft.Extensions.Primitives/src/StringSegment.cs

Co-authored-by: Günther Foidl <gue@korporal.at>

* Add using

* Update StringSegmentTest.cs

* Equals never throws

* Use expression body

* Fix StringSegment_CompareEqual_Globalized

* Fix StringSegment_CompareEqual_Globalized

* Add more tests

* Fix tests

Co-authored-by: Günther Foidl <gue@korporal.at>
  • Loading branch information
@maxkoshevoi @gfoidl
maxkoshevoi and gfoidl authored Aug 24, 2021
1 parent 65f795f commit 4a7a49b
Show file tree
Hide file tree
Showing 3 changed files with 183 additions and 264 deletions.
8 changes: 4 additions & 4 deletions src/libraries/Microsoft.Extensions.Primitives/ref/Microsoft.Extensions.Primitives.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,14 @@ public partial interface IChangeToken
bool HasChanged { get; }
System.IDisposable RegisterChangeCallback(System.Action<object?> callback, object? state);
}
public readonly partial struct StringSegment : System.IEquatable<Microsoft.Extensions.Primitives.StringSegment>, System.IEquatable<string>
public readonly partial struct StringSegment : System.IEquatable<Microsoft.Extensions.Primitives.StringSegment>, System.IEquatable<string?>
{
private readonly object _dummy;
private readonly int _dummyPrimitive;
public static readonly Microsoft.Extensions.Primitives.StringSegment Empty;
public StringSegment(string buffer) { throw null; }
public StringSegment(string? buffer) { throw null; }
public StringSegment(string buffer, int offset, int length) { throw null; }
public string Buffer { get { throw null; } }
public string? Buffer { get { throw null; } }
[System.Diagnostics.CodeAnalysis.MemberNotNullWhen(true, nameof(Buffer))]
public bool HasValue { get { throw null; } }
public char this[int index] { get { throw null; } }
Expand Down Expand Up @@ -74,7 +74,7 @@ public partial interface IChangeToken
public static bool operator ==(Microsoft.Extensions.Primitives.StringSegment left, Microsoft.Extensions.Primitives.StringSegment right) { throw null; }
public static implicit operator System.ReadOnlyMemory<char>(Microsoft.Extensions.Primitives.StringSegment segment) { throw null; }
public static implicit operator System.ReadOnlySpan<char>(Microsoft.Extensions.Primitives.StringSegment segment) { throw null; }
public static implicit operator Microsoft.Extensions.Primitives.StringSegment(string value) { throw null; }
public static implicit operator Microsoft.Extensions.Primitives.StringSegment(string? value) { throw null; }
public static bool operator !=(Microsoft.Extensions.Primitives.StringSegment left, Microsoft.Extensions.Primitives.StringSegment right) { throw null; }
public Microsoft.Extensions.Primitives.StringTokenizer Split(char[] chars) { throw null; }
public bool StartsWith(string text, System.StringComparison comparisonType) { throw null; }
Expand Down
38 changes: 10 additions & 28 deletions src/libraries/Microsoft.Extensions.Primitives/src/StringSegment.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Runtime.CompilerServices;

Expand All @@ -10,7 +11,7 @@ namespace Microsoft.Extensions.Primitives
/// <summary>
/// An optimized representation of a substring.
/// </summary>
public readonly struct StringSegment : IEquatable<StringSegment>, IEquatable<string>
public readonly struct StringSegment : IEquatable<StringSegment>, IEquatable<string?>
{
/// <summary>
/// A <see cref="StringSegment"/> for <see cref="string.Empty"/>.
Expand All @@ -23,7 +24,7 @@ namespace Microsoft.Extensions.Primitives
/// <param name="buffer">
/// The original <see cref="string"/>. The <see cref="StringSegment"/> includes the whole <see cref="string"/>.
/// </param>
public StringSegment(string buffer)
public StringSegment(string? buffer)
{
Buffer = buffer;
Offset = 0;
Expand Down Expand Up @@ -62,7 +63,7 @@ public StringSegment(string buffer, int offset, int length)
/// <summary>
/// Gets the <see cref="string"/> buffer for this <see cref="StringSegment"/>.
/// </summary>
public string Buffer { get; }
public string? Buffer { get; }

/// <summary>
/// Gets the offset within the buffer for this <see cref="StringSegment"/>.
Expand Down Expand Up @@ -102,6 +103,7 @@ public char this[int index]
ThrowHelper.ThrowArgumentOutOfRangeException(ExceptionArgument.index);
}

Debug.Assert(Buffer is not null);
return Buffer[Offset + index];
}
}
Expand Down Expand Up @@ -231,20 +233,14 @@ public bool Equals(StringSegment other, StringComparison comparisonType)
/// <param name="b">The second <see cref="StringSegment"/> to compare.</param>
/// <param name="comparisonType">One of the enumeration values that specifies the rules for the comparison.</param>
/// <returns><see langword="true" /> if the objects are equal; otherwise, <see langword="false" />.</returns>
public static bool Equals(StringSegment a, StringSegment b, StringComparison comparisonType)
{
return a.Equals(b, comparisonType);
}
public static bool Equals(StringSegment a, StringSegment b, StringComparison comparisonType) => a.Equals(b, comparisonType);

/// <summary>
/// Checks if the specified <see cref="string"/> is equal to the current <see cref="StringSegment"/>.
/// </summary>
/// <param name="text">The <see cref="string"/> to compare with the current <see cref="StringSegment"/>.</param>
/// <returns><see langword="true" /> if the specified <see cref="string"/> is equal to the current <see cref="StringSegment"/>; otherwise, <see langword="false" />.</returns>
public bool Equals([NotNullWhen(true)] string? text)
{
return Equals(text, StringComparison.Ordinal);
}
public bool Equals([NotNullWhen(true)] string? text) => Equals(text, StringComparison.Ordinal);

/// <summary>
/// Checks if the specified <see cref="string"/> is equal to the current <see cref="StringSegment"/>.
Expand All @@ -255,15 +251,10 @@ public bool Equals([NotNullWhen(true)] string? text)
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public bool Equals([NotNullWhen(true)] string? text, StringComparison comparisonType)
{
if (text == null)
{
return false;
}

if (!HasValue)
if (!HasValue || text == null)
{
CheckStringComparison(comparisonType); // must arg check before returning
return false;
return text == Buffer; // return true if both are null
}

return AsSpan().Equals(text.AsSpan(), comparisonType);
Expand Down Expand Up @@ -311,7 +302,7 @@ public override int GetHashCode()
/// Creates a new <see cref="StringSegment"/> from the given <see cref="string"/>.
/// </summary>
/// <param name="value">The <see cref="string"/> to convert to a <see cref="StringSegment"/></param>
public static implicit operator StringSegment(string value) => new StringSegment(value);
public static implicit operator StringSegment(string? value) => new StringSegment(value);

/// <summary>
/// Creates a see <see cref="ReadOnlySpan{T}"/> from the given <see cref="StringSegment"/>.
Expand Down Expand Up @@ -732,14 +723,5 @@ Exception GetInvalidArgumentsException(bool hasValue)
return ThrowHelper.GetArgumentException(ExceptionResource.Argument_InvalidOffsetLengthStringSegment);
}
}

/// <inheritdoc />
bool IEquatable<string>.Equals(string? other)
{
// Explicit interface implementation for IEquatable<string> because
// the interface's Equals method allows null strings, which we return
// as not-equal.
return other != null && Equals(other);
}
}
}
Loading

0 comments on commit 4a7a49b

Please sign in to comment.