Skip to content

Commit

Permalink
Feature/always strict (#8)
Browse files Browse the repository at this point in the history
* Reduce surface area that exposes `strict` options. Strict is now always on.
  • Loading branch information
davetransom authored Mar 11, 2021
1 parent c57c98b commit 835267f
Show file tree
Hide file tree
Showing 4 changed files with 136 additions and 246 deletions.
35 changes: 18 additions & 17 deletions CSharpVitamins.ShortGuid.Tests/ShortGuidFacts.cs
Original file line number Diff line number Diff line change
Expand Up @@ -38,55 +38,52 @@ 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<ArgumentException>(
() => ShortGuid.Decode(LongerBase64String, strict: false)
() => ShortGuid.Decode(LongerBase64String)
);
}

[Fact]
void StrictDecode_does_not_parse_longer_base64_string()
{
Assert.Throws<ArgumentException>(
() => 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<FormatException>(
() => 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<FormatException>(
() => new ShortGuid(InvalidSampleShortGuidString)
);
}

[Fact]
void ctor_throws_when_trying_to_decode_guid_string()
{
Assert.Throws<FormatException>(
Assert.Throws<ArgumentException>(
() => new ShortGuid(SampleGuidString)
);
}
Expand Down Expand Up @@ -153,9 +150,13 @@ void Decode_takes_expected_string()
[Fact]
void Decode_fails_on_unexpected_string()
{
Assert.Throws<FormatException>(
Assert.Throws<ArgumentException>(
() => ShortGuid.Decode("Am I valid?")
);

Assert.Throws<FormatException>(
() => ShortGuid.Decode("I am 22characters long")
);
}

[Fact]
Expand Down
24 changes: 21 additions & 3 deletions CSharpVitamins.ShortGuid/CSharpVitamins.ShortGuid.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,31 @@
<RootNamespace>CSharpVitamins</RootNamespace>
<Authors>Dave Transom</Authors>
<Company>CSharpVitamins</Company>
<Description>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).</Description>
<Description>
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.
</Description>
<PackageLicenseFile>LICENSE.md</PackageLicenseFile>
<PackageProjectUrl>https://github.com/csharpvitamins/csharpvitamins.shortguid/</PackageProjectUrl>
<PackageLicenseExpression></PackageLicenseExpression>
<RepositoryUrl>https://github.com/csharpvitamins/csharpvitamins.shortguid/</RepositoryUrl>
<PackageTags>Guid ShortGuid Identifiers Base64</PackageTags>
<PackageTags>Guid ShortGuid Identifiers Base64 CSharpVitamins</PackageTags>
<PackageReleaseNotes>
- 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.
Expand All @@ -22,7 +40,7 @@
- 1.0.0 Initial release.
</PackageReleaseNotes>
<Copyright>Copyright 2007</Copyright>
<Version>1.1.0</Version>
<Version>2.0.0</Version>
<PublishRepositoryUrl>true</PublishRepositoryUrl>
<IncludeSymbols>true</IncludeSymbols>
<SymbolPackageFormat>snupkg</SymbolPackageFormat>
Expand Down
Loading

0 comments on commit 835267f

Please sign in to comment.