From 835267feaf85da0651293c7295d5eb2e9f012990 Mon Sep 17 00:00:00 2001 From: Dave Transom Date: Fri, 12 Mar 2021 00:08:00 +1300 Subject: [PATCH] Feature/always strict (#8) * Reduce surface area that exposes `strict` options. Strict is now always on. --- .../ShortGuidFacts.cs | 35 +- .../CSharpVitamins.ShortGuid.csproj | 24 +- CSharpVitamins.ShortGuid/ShortGuid.cs | 311 +++++------------- readme.md | 12 + 4 files changed, 136 insertions(+), 246 deletions(-) diff --git a/CSharpVitamins.ShortGuid.Tests/ShortGuidFacts.cs b/CSharpVitamins.ShortGuid.Tests/ShortGuidFacts.cs index c3e6a2a..e90199c 100644 --- a/CSharpVitamins.ShortGuid.Tests/ShortGuidFacts.cs +++ b/CSharpVitamins.ShortGuid.Tests/ShortGuidFacts.cs @@ -38,20 +38,20 @@ void ctor_decodes_shortguid_string() [Fact] void StrictDecode_parses_valid_shortGuid_strict_off() { - ShortGuid.Decode(SampleShortGuidString, strict: false); + ShortGuid.Decode(SampleShortGuidString); } [Fact] void StrictDecode_parses_valid_shortGuid_strict_on() { - ShortGuid.Decode(SampleShortGuidString, strict: true); + ShortGuid.Decode(SampleShortGuidString); } [Fact] void Decode_does_not_parse_longer_base64_string() { Assert.Throws( - () => ShortGuid.Decode(LongerBase64String, strict: false) + () => ShortGuid.Decode(LongerBase64String) ); } @@ -59,34 +59,31 @@ void Decode_does_not_parse_longer_base64_string() void StrictDecode_does_not_parse_longer_base64_string() { Assert.Throws( - () => ShortGuid.Decode(LongerBase64String, strict: true) + () => ShortGuid.Decode(LongerBase64String) ); } [Fact] void invalid_strings_must_not_return_true_on_try_parse_with_strict_true() { - // loose parsing should return true - Assert.True(ShortGuid.TryParse(InvalidSampleShortGuidString, out ShortGuid looseSguid, strict: false)); + // try parse should return false + Assert.False(ShortGuid.TryParse(InvalidSampleShortGuidString, out ShortGuid strictSguid)); - // strict parsing should return false - Assert.False(ShortGuid.TryParse(InvalidSampleShortGuidString, out ShortGuid strictSguid, strict: true)); - - // decode with strict should throw + // decode should throw Assert.Throws( - () => ShortGuid.Decode(InvalidSampleShortGuidString, strict: true) + () => ShortGuid.Decode(InvalidSampleShortGuidString) ); - // does not throw an exception because strict defaults to false - Guid looseDecodedSguid = ShortGuid.Decode(InvalidSampleShortGuidString); - - Assert.Equal((Guid)looseSguid, looseDecodedSguid); + // .ctor should throw + Assert.Throws( + () => new ShortGuid(InvalidSampleShortGuidString) + ); } [Fact] void ctor_throws_when_trying_to_decode_guid_string() { - Assert.Throws( + Assert.Throws( () => new ShortGuid(SampleGuidString) ); } @@ -153,9 +150,13 @@ void Decode_takes_expected_string() [Fact] void Decode_fails_on_unexpected_string() { - Assert.Throws( + Assert.Throws( () => ShortGuid.Decode("Am I valid?") ); + + Assert.Throws( + () => ShortGuid.Decode("I am 22characters long") + ); } [Fact] diff --git a/CSharpVitamins.ShortGuid/CSharpVitamins.ShortGuid.csproj b/CSharpVitamins.ShortGuid/CSharpVitamins.ShortGuid.csproj index c8f5dba..9998910 100644 --- a/CSharpVitamins.ShortGuid/CSharpVitamins.ShortGuid.csproj +++ b/CSharpVitamins.ShortGuid/CSharpVitamins.ShortGuid.csproj @@ -6,13 +6,31 @@ CSharpVitamins Dave Transom CSharpVitamins - A convenience wrapper struct for dealing with URL-safe Base64 encoded globally unique identifiers (GUID), making a shorter string value (22 vs 36 characters long). + +A convenience wrapper struct for dealing with URL-safe Base64 encoded globally unique identifiers (GUID), +making a shorter string value (22 vs 36 characters long). + +As of version 2.0.0, `ShortGuid` performs a sanity check when decoding strings to ensure they haven't been +tampered with, i.e. allowing the end of a Base64 string to be tweaked where it still produces that same +byte array to create the underlying Guid. Effectively there is "unused space" in the Base64 string which is +ignored, but will now result in an `FormatException` being thrown. + +ShortGuid will never produce an invalid string, however if one is supplied, it could result in an unintended +collision where multiple URL-safe Base64 strings can point to the same Guid. To avoid this uncertainty, a +round-trip check is performed to ensure a 1-1 match with the input string. + +Stick with version 1.1.0 if you require the old behaviour with opt-in strict parsing. + LICENSE.md https://github.com/csharpvitamins/csharpvitamins.shortguid/ https://github.com/csharpvitamins/csharpvitamins.shortguid/ - Guid ShortGuid Identifiers Base64 + Guid ShortGuid Identifiers Base64 CSharpVitamins +- 2.0.0 Strict is now always on. + Reduces surface area that exposed `strict` parsing options. + Stick with `v1.1.0` if you require the loose form of decoding. + - 1.1.0 Adds overloads for strict parsing to (Try)Decode/Parse methods. Default: strict=false for backwards compatibility. Signals intent to move to strict only parsing in version 2+. - 1.0.3 Improves documentation. @@ -22,7 +40,7 @@ - 1.0.0 Initial release. Copyright 2007 - 1.1.0 + 2.0.0 true true snupkg diff --git a/CSharpVitamins.ShortGuid/ShortGuid.cs b/CSharpVitamins.ShortGuid/ShortGuid.cs index 1ce2c18..51972cd 100644 --- a/CSharpVitamins.ShortGuid/ShortGuid.cs +++ b/CSharpVitamins.ShortGuid/ShortGuid.cs @@ -10,6 +10,18 @@ namespace CSharpVitamins /// /// What is URL-safe Base64? That's just a Base64 string with well known special characters replaced (/, +) /// or removed (==). + /// + /// NB: As of version 2.0.0, performs a sanity check when decoding + /// strings to ensure they haven't been tampered with, i.e. allowing the end of a Base64 string to be tweaked + /// where it still produces that same byte array to create the underlying Guid. Effectively there is "unused + /// space" in the Base64 string which is ignored, but will now result in an being + /// thrown. + /// + /// ShortGuid will never produce an invalid string, however if one is supplied it, could result in an + /// unintended collision where multiple URL-safe Base64 strings can point to the same Guid. To avoid this + /// uncertainty, a round-trip check is performed to ensure a 1-1 match with the input string. + /// + /// Stick with version 1.1.0 if you require the old behaviour with opt-in strict parsing. /// [DebuggerDisplay("{Value}")] public struct ShortGuid @@ -20,42 +32,21 @@ public struct ShortGuid /// public static readonly ShortGuid Empty = new ShortGuid(Guid.Empty); - Guid underlyingGuid; - string encodedString; + readonly Guid underlyingGuid; + readonly string encodedString; /// /// Creates a new instance with the given URL-safe Base64 encoded string. /// See also which will try to coerce the /// the value from URL-safe Base64 or normal Guid string. - /// - /// NB: This method does not check if the result Guid is strictly correct. It accepts any Base64 encoded - /// string, allowing a) longer strings to be parsed where the remaining data is ignored and b) the end of a - /// Base64 string to be tweaked where it still produces that same byte array to create the underlying Guid. - /// Effectively there is "unused space" in the Base64 string which is ignored. - /// - /// See the strict version of this method which ensures the value being passed in is valid. Strict decoding - /// will be on by default from version 2+. /// - /// A ShortGuid encoded string e.g. URL-safe Base64. + /// A 22 character URL-safe Base64 encoded string to decode. public ShortGuid(string value) { encodedString = value; underlyingGuid = Decode(value); } - /// - /// Creates a new instance with the given URL-safe Base64 encoded string. - /// See also which will try to coerce the - /// the value from URL-safe Base64 or normal Guid string. - /// - /// A ShortGuid encoded string e.g. URL-safe Base64. - /// /// If true the re-encoded result has to exactly match the input ; if false any valid base64 string will be accepted. - public ShortGuid(string value, bool strict) - { - encodedString = value; - underlyingGuid = Decode(value, strict); - } - /// /// Creates a new instance with the given . /// @@ -72,7 +63,7 @@ public ShortGuid(Guid guid) public Guid Guid => underlyingGuid; /// - /// Gets the encoded string value of the i.e. a URL-safe Base64 string. + /// Gets the encoded string value of the as a URL-safe Base64 string. /// public string Value => encodedString; @@ -103,7 +94,7 @@ public override bool Equals(object obj) if (obj is string str) { // Try a ShortGuid string. - if (TryDecode(str, out guid, strict: false)) + if (TryDecode(str, out guid)) return underlyingGuid.Equals(guid); // Try a guid string. @@ -158,44 +149,15 @@ public static string Encode(Guid guid) /// /// Decodes the given value to a . /// See also or . - /// - /// NB: This method does not check if the result Guid is strictly correct. It accepts any Base64 encoded - /// string, allowing a) longer strings to be parsed where the remaining data is ignored and b) the end of a - /// Base64 string to be tweaked where it still produces that same byte array to create the underlying Guid. - /// Effectively there is "unused space" in the Base64 string which is ignored. - /// - /// See the strict version of this method which ensures the value being passed in is valid. Strict decoding - /// will be on by default from version 2+. - /// - /// A 22 character URL-safe Base64 encoded string to decode. - /// A new instance from the parsed string. - public static Guid Decode(string value) - { - string base64 = value - .Replace("_", "/") - .Replace("-", "+") + "=="; - - byte[] blob = Convert.FromBase64String(base64); - return new Guid(blob); - } - - /// - /// Decodes the given value to a . - /// See also or . /// /// A 22 character URL-safe Base64 encoded string to decode. - /// If true the re-encoded result has to exactly match the input ; if false any valid base64 string will be accepted. /// A new instance from the parsed string. /// /// If is not a valid Base64 string () - /// or if the flag is set and the decoded guid doesn't strictly match the input . + /// or if the decoded guid doesn't strictly match the input . /// - public static Guid Decode(string value, bool strict) + public static Guid Decode(string value) { - // if not strict, defer to the older non-strict version. - if (!strict) - return Decode(value); - // avoid parsing larger strings/blobs if (value?.Length != 22) { @@ -242,56 +204,99 @@ public static Guid Decode(string value, bool strict) /// Tries to parse as a only, but outputs the result as a - this method. /// /// - /// - /// NB: This method does not check if the result Guid is strictly correct. It accepts any Base64 encoded - /// string, allowing a) longer strings to be parsed where the remaining data is ignored and b) the end of a - /// Base64 string to be tweaked where it still produces that same byte array to create the underlying Guid. - /// Effectively there is "unused space" in the Base64 string which is ignored. - /// /// /// The ShortGuid encoded string to decode. /// A new instance from the parsed string. /// A boolean indicating if the decode was successful. public static bool TryDecode(string value, out Guid guid) { - return TryDecode(value, out guid, strict: false); + try + { + guid = Decode(value); + return true; + } + catch + { + guid = Guid.Empty; + return false; + } } /// - /// Attempts to decode the given value to a . + /// Tries to parse the value as a or string, and outputs an actual instance. /// /// The difference between TryParse and TryDecode: /// /// - /// - /// Tries to parse as a before attempting parsing as a , outputs the actual instance. + /// + /// Tries to parse as a before attempting parsing as a , outputs the actual instance - this method. /// /// - /// + /// /// Tries to parse as a before attempting parsing as a , outputs the underlying . /// /// - /// - /// Tries to parse as a only, but outputs the result as a - this method. + /// + /// Tries to parse as a only, but outputs the result as a . /// /// /// - /// The ShortGuid encoded string to decode. - /// A new instance from the parsed string. - /// If true the re-encoded result has to exactly match the input ; if false any valid base64 string will be accepted. - /// A boolean indicating if the decode was successful. - public static bool TryDecode(string value, out Guid guid, bool strict) + /// The ShortGuid encoded string or string representation of a Guid. + /// A new instance from the parsed string. + public static bool TryParse(string value, out ShortGuid shortGuid) { - try + // Try a ShortGuid string. + if (ShortGuid.TryDecode(value, out var guid)) { - guid = Decode(value, strict); + shortGuid = guid; return true; } - catch + + // Try a Guid string. + if (Guid.TryParse(value, out guid)) { - guid = Guid.Empty; - return false; + shortGuid = guid; + return true; } + + shortGuid = ShortGuid.Empty; + return false; + } + + /// + /// Tries to parse the value as a or string, and outputs the underlying value. + /// + /// The difference between TryParse and TryDecode: + /// + /// + /// + /// Tries to parse as a before attempting parsing as a , outputs the actual instance. + /// + /// + /// + /// Tries to parse as a before attempting parsing as a , outputs the underlying - this method. + /// + /// + /// + /// Tries to parse as a only, but outputs the result as a . + /// + /// + /// + /// The ShortGuid encoded string or string representation of a Guid. + /// A new instance from the parsed string. + /// A boolean indicating if the parse was successful. + public static bool TryParse(string value, out Guid guid) + { + // Try a ShortGuid string. + if (ShortGuid.TryDecode(value, out guid)) + return true; + + // Try a Guid string. + if (Guid.TryParse(value, out guid)) + return true; + + guid = Guid.Empty; + return false; } #region Operators @@ -375,151 +380,5 @@ public static implicit operator ShortGuid(Guid guid) } #endregion - - /// - /// Tries to parse the value as a or string, and outputs an actual instance. - /// - /// The difference between TryParse and TryDecode: - /// - /// - /// - /// Tries to parse as a before attempting parsing as a , outputs the actual instance - this method. - /// - /// - /// - /// Tries to parse as a before attempting parsing as a , outputs the underlying . - /// - /// - /// - /// Tries to parse as a only, but outputs the result as a . - /// - /// - /// - /// NB: This method does not check if the result Guid is strictly correct. It accepts any Base64 encoded - /// string, allowing a) longer strings to be parsed where the remaining data is ignored and b) the end of a - /// Base64 string to be tweaked where it still produces that same byte array to create the underlying Guid. - /// Effectively there is "unused space" in the Base64 string which is ignored. - /// - /// - /// The ShortGuid encoded string or string representation of a Guid. - /// A new instance from the parsed string. - /// A boolean indicating if the parse was successful. - public static bool TryParse(string value, out ShortGuid shortGuid) - { - return TryParse(value, out shortGuid, strict: false); - } - - /// - /// Tries to parse the value as a or string, and outputs an actual instance. - /// - /// The difference between TryParse and TryDecode: - /// - /// - /// - /// Tries to parse as a before attempting parsing as a , outputs the actual instance - this method. - /// - /// - /// - /// Tries to parse as a before attempting parsing as a , outputs the underlying . - /// - /// - /// - /// Tries to parse as a only, but outputs the result as a . - /// - /// - /// - /// The ShortGuid encoded string or string representation of a Guid. - /// A new instance from the parsed string. - /// If true the re-encoded result has to exactly match the input ; if false any valid base64 string will be accepted. - /// A boolean indicating if the parse was successful. - public static bool TryParse(string value, out ShortGuid shortGuid, bool strict) - { - // Try a ShortGuid string. - if (ShortGuid.TryDecode(value, out var guid, strict)) - { - shortGuid = guid; - return true; - } - - // Try a Guid string. - if (Guid.TryParse(value, out guid)) - { - shortGuid = guid; - return true; - } - - shortGuid = ShortGuid.Empty; - return false; - } - - /// - /// Tries to parse the value as a or string, and outputs the underlying value. - /// - /// The difference between TryParse and TryDecode: - /// - /// - /// - /// Tries to parse as a before attempting parsing as a , outputs the actual instance. - /// - /// - /// - /// Tries to parse as a before attempting parsing as a , outputs the underlying - this method. - /// - /// - /// - /// Tries to parse as a only, but outputs the result as a . - /// - /// - /// - /// NB: This method does not check if the result Guid is strictly correct. It accepts any Base64 encoded - /// string, allowing a) longer strings to be parsed where the remaining data is ignored and b) the end of a - /// Base64 string to be tweaked where it still produces that same byte array to create the underlying Guid. - /// Effectively there is "unused space" in the Base64 string which is ignored. - /// - /// - /// The ShortGuid encoded string or string representation of a Guid. - /// A new instance from the parsed string. - /// A boolean indicating if the parse was successful. - public static bool TryParse(string value, out Guid guid) - { - return TryParse(value, out guid, strict: false); - } - - /// - /// Tries to parse the value as a or string, and outputs the underlying value. - /// - /// The difference between TryParse and TryDecode: - /// - /// - /// - /// Tries to parse as a before attempting parsing as a , outputs the actual instance. - /// - /// - /// - /// Tries to parse as a before attempting parsing as a , outputs the underlying - this method. - /// - /// - /// - /// Tries to parse as a only, but outputs the result as a . - /// - /// - /// - /// The ShortGuid encoded string or string representation of a Guid. - /// A new instance from the parsed string. - /// If true the re-encoded result has to exactly match the input ; if false any valid base64 string will be accepted. - /// A boolean indicating if the parse was successful. - public static bool TryParse(string value, out Guid guid, bool strict) - { - // Try a ShortGuid string. - if (ShortGuid.TryDecode(value, out guid, strict)) - return true; - - // Try a Guid string. - if (Guid.TryParse(value, out guid)) - return true; - - guid = Guid.Empty; - return false; - } } } diff --git a/readme.md b/readme.md index 601836a..59d203d 100644 --- a/readme.md +++ b/readme.md @@ -10,7 +10,19 @@ Available on [NuGet](https://www.nuget.org/packages/csharpvitamins.shortguid/). PM> Install-Package CSharpVitamins.ShortGuid +### Strict Parsing (v2.0.0) +As of version 2.0.0, `ShortGuid` performs a sanity check when decoding strings to ensure they haven't been +tampered with, i.e. allowing the end of a Base64 string to be tweaked where it still produces that same +byte array to create the underlying Guid. Effectively there is "unused space" in the Base64 string which is +ignored, but will now result in an `FormatException` being thrown. +`ShortGuid` will never produce an invalid string, however if one is supplied, it could result in an unintended +collision where multiple URL-safe Base64 strings can point to the same Guid. To avoid this uncertainty, a +round-trip check is performed to ensure a 1-1 match with the input string. + +Stick with version 1.1.0 if you require the old behaviour with opt-in strict parsing. + +--- ## The Gist