From e42de8349f6af7754de39ecd508a7f73356c27a7 Mon Sep 17 00:00:00 2001 From: Hao Kung Date: Tue, 6 Jun 2017 11:37:00 -0700 Subject: [PATCH 1/5] Harden the PathString converter --- .../PathString.cs | 17 ++++++++++++++--- .../PathStringTests.cs | 9 ++++++++- 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/src/Microsoft.AspNetCore.Http.Abstractions/PathString.cs b/src/Microsoft.AspNetCore.Http.Abstractions/PathString.cs index b82d4442..59bc502d 100644 --- a/src/Microsoft.AspNetCore.Http.Abstractions/PathString.cs +++ b/src/Microsoft.AspNetCore.Http.Abstractions/PathString.cs @@ -459,9 +459,20 @@ public static implicit operator string(PathString path) internal class PathStringConverter : TypeConverter { + public override bool CanConvertFrom(ITypeDescriptorContext context, Type sourceType) + => sourceType == typeof(string) + ? true + : base.CanConvertFrom(context, sourceType); + public override object ConvertFrom(ITypeDescriptorContext context, CultureInfo culture, object value) - { - return new PathString((string)value); - } + => value is string + ? new PathString((string)value) + : base.ConvertFrom(context, culture, value); + + public override object ConvertTo(ITypeDescriptorContext context, + CultureInfo culture, object value, Type destinationType) + => destinationType == typeof(string) + ? value.ToString() + : base.ConvertTo(context, culture, value, destinationType); } } diff --git a/test/Microsoft.AspNetCore.Http.Abstractions.Tests/PathStringTests.cs b/test/Microsoft.AspNetCore.Http.Abstractions.Tests/PathStringTests.cs index 365a9de3..4d8edde7 100644 --- a/test/Microsoft.AspNetCore.Http.Abstractions.Tests/PathStringTests.cs +++ b/test/Microsoft.AspNetCore.Http.Abstractions.Tests/PathStringTests.cs @@ -208,11 +208,18 @@ public void ToUriComponentEscapeCorrectly(string category, string input, string } [Fact] - public void PathStringConvertsFromString() + public void PathStringConvertsOnlyToAndFromString() { var converter = TypeDescriptor.GetConverter(typeof(PathString)); PathString result = (PathString)converter.ConvertFromInvariantString("/foo"); Assert.Equal("/foo", result.ToString()); + Assert.Equal("/foo", converter.ConvertTo(result, typeof(string))); + Assert.True(converter.CanConvertFrom(typeof(string))); + Assert.False(converter.CanConvertFrom(typeof(int))); + Assert.False(converter.CanConvertFrom(typeof(bool))); + Assert.True(converter.CanConvertTo(typeof(string))); + Assert.False(converter.CanConvertTo(typeof(int))); + Assert.False(converter.CanConvertTo(typeof(bool))); } } } From d063a1f653c6ab7c66bebd7f6aa06bd28b56df02 Mon Sep 17 00:00:00 2001 From: Hao Kung Date: Tue, 6 Jun 2017 12:54:07 -0700 Subject: [PATCH 2/5] Unify ConvertFromString --- src/Microsoft.AspNetCore.Http.Abstractions/PathString.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Microsoft.AspNetCore.Http.Abstractions/PathString.cs b/src/Microsoft.AspNetCore.Http.Abstractions/PathString.cs index 59bc502d..39ed78b4 100644 --- a/src/Microsoft.AspNetCore.Http.Abstractions/PathString.cs +++ b/src/Microsoft.AspNetCore.Http.Abstractions/PathString.cs @@ -443,9 +443,6 @@ public override int GetHashCode() /// /// public static implicit operator PathString(string s) - { - return new PathString(s); - } /// /// Implicitly calls ToString(). @@ -455,6 +452,9 @@ public static implicit operator string(PathString path) { return path.ToString(); } + + internal static PathString ConvertFromString(string s) + => string.IsNullOrEmpty(s) ? new PathString(s) : FromUriComponent(s); } internal class PathStringConverter : TypeConverter @@ -466,7 +466,7 @@ public override bool CanConvertFrom(ITypeDescriptorContext context, Type sourceT public override object ConvertFrom(ITypeDescriptorContext context, CultureInfo culture, object value) => value is string - ? new PathString((string)value) + ? PathString.ConvertFromString((string)value) : base.ConvertFrom(context, culture, value); public override object ConvertTo(ITypeDescriptorContext context, From 1ba096f66dfb5ae6cbf94d4d67e3f96cec7f5813 Mon Sep 17 00:00:00 2001 From: Hao Kung Date: Tue, 6 Jun 2017 12:54:14 -0700 Subject: [PATCH 3/5] Fix --- src/Microsoft.AspNetCore.Http.Abstractions/PathString.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Microsoft.AspNetCore.Http.Abstractions/PathString.cs b/src/Microsoft.AspNetCore.Http.Abstractions/PathString.cs index 39ed78b4..3ccc2cb2 100644 --- a/src/Microsoft.AspNetCore.Http.Abstractions/PathString.cs +++ b/src/Microsoft.AspNetCore.Http.Abstractions/PathString.cs @@ -443,6 +443,7 @@ public override int GetHashCode() /// /// public static implicit operator PathString(string s) + => ConvertFromString(s); /// /// Implicitly calls ToString(). From 86a5c05f0d9e92ab293a34896db1fdcea98fb55b Mon Sep 17 00:00:00 2001 From: Hao Kung Date: Thu, 15 Jun 2017 15:53:21 -0700 Subject: [PATCH 4/5] Add test --- .../PathString.cs | 4 +--- .../PathStringTests.cs | 12 +++++++++++- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/src/Microsoft.AspNetCore.Http.Abstractions/PathString.cs b/src/Microsoft.AspNetCore.Http.Abstractions/PathString.cs index 3ccc2cb2..2a5960b6 100644 --- a/src/Microsoft.AspNetCore.Http.Abstractions/PathString.cs +++ b/src/Microsoft.AspNetCore.Http.Abstractions/PathString.cs @@ -450,9 +450,7 @@ public static implicit operator PathString(string s) /// /// public static implicit operator string(PathString path) - { - return path.ToString(); - } + => path.ToString(); internal static PathString ConvertFromString(string s) => string.IsNullOrEmpty(s) ? new PathString(s) : FromUriComponent(s); diff --git a/test/Microsoft.AspNetCore.Http.Abstractions.Tests/PathStringTests.cs b/test/Microsoft.AspNetCore.Http.Abstractions.Tests/PathStringTests.cs index 4d8edde7..a321e43b 100644 --- a/test/Microsoft.AspNetCore.Http.Abstractions.Tests/PathStringTests.cs +++ b/test/Microsoft.AspNetCore.Http.Abstractions.Tests/PathStringTests.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; @@ -221,5 +221,15 @@ public void PathStringConvertsOnlyToAndFromString() Assert.False(converter.CanConvertTo(typeof(int))); Assert.False(converter.CanConvertTo(typeof(bool))); } + + [Fact] + public void PathStringStaysEqualAfterAssignments() + { + PathString p1 = "/?"; + string s1 = p1; // Becomes "/%3F" + PathString p2 = s1; // Stays "/%3F" + Assert.Equal(p1, p2); + } + } } From 052464535056b724550092f9da9c68689027d22b Mon Sep 17 00:00:00 2001 From: Hao Kung Date: Fri, 16 Jun 2017 14:09:03 -0700 Subject: [PATCH 5/5] Update string comment --- .../PathStringTests.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/test/Microsoft.AspNetCore.Http.Abstractions.Tests/PathStringTests.cs b/test/Microsoft.AspNetCore.Http.Abstractions.Tests/PathStringTests.cs index a321e43b..ad070afa 100644 --- a/test/Microsoft.AspNetCore.Http.Abstractions.Tests/PathStringTests.cs +++ b/test/Microsoft.AspNetCore.Http.Abstractions.Tests/PathStringTests.cs @@ -226,10 +226,9 @@ public void PathStringConvertsOnlyToAndFromString() public void PathStringStaysEqualAfterAssignments() { PathString p1 = "/?"; - string s1 = p1; // Becomes "/%3F" - PathString p2 = s1; // Stays "/%3F" + string s1 = p1; + PathString p2 = s1; Assert.Equal(p1, p2); } - } }