diff options
author | Eric Mellino <erme@microsoft.com> | 2017-08-05 02:30:12 +0300 |
---|---|---|
committer | GitHub <noreply@github.com> | 2017-08-05 02:30:12 +0300 |
commit | be95f4f0da2d11e5508f2554b8b7633c02ed24f6 (patch) | |
tree | ff7d0b3322a4ea2bbb39911d41ddce173f8dc036 | |
parent | b3dfe02df8dc13473afc2f739cfd0ebc3c4b3a4f (diff) |
Clean up path validation related issues in System.Drawing.Common (#22941)
* Allow different exception types between netfx and netcoreapp in a Metafile test.
* Fix two ImageAttributesTests.
* These tests were failing due to a missing call to Path.GetFullPath
(effectively).
* I have added the call to Path.GetFullPath back into the implementation
assembly.
* The second test has been removed because it relied on Path.GetFullPath
throwing a PathTooLongException, which is no longer the case in .NET
Core.
* Add missing call to Path.GetFullPath in Metafile constructor.
Also, fix a test related to this issue.
* Add more missing calls to Path.GetFullPath in Metafile.
Fix test validation for these paths.
* Add back PathTooLongException tests.
4 files changed, 26 insertions, 28 deletions
diff --git a/src/System.Drawing.Common/src/System/Drawing/Imaging/ImageAttributes.cs b/src/System.Drawing.Common/src/System/Drawing/Imaging/ImageAttributes.cs index b276d495ad..431201ee11 100644 --- a/src/System.Drawing.Common/src/System/Drawing/Imaging/ImageAttributes.cs +++ b/src/System.Drawing.Common/src/System/Drawing/Imaging/ImageAttributes.cs @@ -6,6 +6,7 @@ using System.Runtime.InteropServices; using System.Diagnostics; using System.Drawing.Drawing2D; using System.Globalization; +using System.IO; namespace System.Drawing.Imaging { @@ -405,6 +406,9 @@ namespace System.Drawing.Imaging public void SetOutputChannelColorProfile(String colorProfileFilename, ColorAdjustType type) { + // Called in order to emulate exception behavior from netfx related to invalid file paths. + Path.GetFullPath(colorProfileFilename); + int status = SafeNativeMethods.Gdip.GdipSetImageAttributesOutputChannelColorProfile( new HandleRef(this, nativeImageAttributes), type, diff --git a/src/System.Drawing.Common/src/System/Drawing/Imaging/Metafile.cs b/src/System.Drawing.Common/src/System/Drawing/Imaging/Metafile.cs index 53f07e73dd..e043f81811 100644 --- a/src/System.Drawing.Common/src/System/Drawing/Imaging/Metafile.cs +++ b/src/System.Drawing.Common/src/System/Drawing/Imaging/Metafile.cs @@ -59,6 +59,9 @@ namespace System.Drawing.Imaging /// </summary> public Metafile(string filename) { + // Called in order to emulate exception behavior from netfx related to invalid file paths. + Path.GetFullPath(filename); + IntPtr metafile = IntPtr.Zero; int status = SafeNativeMethods.Gdip.GdipCreateMetafileFromFile(filename, out metafile); @@ -240,6 +243,9 @@ namespace System.Drawing.Imaging /// </summary> public Metafile(string fileName, IntPtr referenceHdc, EmfType type, String description) { + // Called in order to emulate exception behavior from netfx related to invalid file paths. + Path.GetFullPath(fileName); + IntPtr metafile = IntPtr.Zero; int status = SafeNativeMethods.Gdip.GdipRecordMetafileFileName(fileName, new HandleRef(null, referenceHdc), @@ -555,6 +561,9 @@ namespace System.Drawing.Imaging /// </summary> public static MetafileHeader GetMetafileHeader(string fileName) { + // Called in order to emulate exception behavior from netfx related to invalid file paths. + Path.GetFullPath(fileName); + MetafileHeader header = new MetafileHeader(); IntPtr memory = Marshal.AllocHGlobal(Marshal.SizeOf(typeof(MetafileHeaderEmf))); diff --git a/src/System.Drawing.Common/tests/Imaging/ImageAttributesTests.cs b/src/System.Drawing.Common/tests/Imaging/ImageAttributesTests.cs index 08047941ad..9b66ee8dfe 100644 --- a/src/System.Drawing.Common/tests/Imaging/ImageAttributesTests.cs +++ b/src/System.Drawing.Common/tests/Imaging/ImageAttributesTests.cs @@ -1191,7 +1191,6 @@ namespace System.Drawing.Imaging.Tests imageAttr.SetOutputChannelColorProfile(Helpers.GetTestColorProfilePath("RSWOP.icm"), ColorAdjustType.Default)); } - [ActiveIssue(22367)] [ConditionalFact(Helpers.GdiplusIsAvailable)] public void SetOutputChannelColorProfile_Null_ThrowsArgumentNullException() { @@ -1223,11 +1222,10 @@ namespace System.Drawing.Imaging.Tests } } - [ActiveIssue(22309)] [ConditionalFact(Helpers.GdiplusIsAvailable)] public void SetOutputChannelColorProfile_InvalidPath_ThrowsPathTooLongException() { - string fileNameTooLong = new string('a', 261); + string fileNameTooLong = new string('a', short.MaxValue); using (var imageAttr = new ImageAttributes()) { Assert.Throws<PathTooLongException>(() => imageAttr.SetOutputChannelColorProfile(fileNameTooLong)); @@ -1235,6 +1233,7 @@ namespace System.Drawing.Imaging.Tests } } + [ConditionalTheory(Helpers.GdiplusIsAvailable)] [MemberData(nameof(ColorAdjustType_InvalidTypes_TestData))] public void SetOutputChannelColorProfile_InvalidTypes_ThrowsArgumentException(ColorAdjustType type) diff --git a/src/System.Drawing.Common/tests/Imaging/MetafileTests.cs b/src/System.Drawing.Common/tests/Imaging/MetafileTests.cs index 79e7d69f48..919b90cb01 100644 --- a/src/System.Drawing.Common/tests/Imaging/MetafileTests.cs +++ b/src/System.Drawing.Common/tests/Imaging/MetafileTests.cs @@ -71,7 +71,6 @@ namespace System.Drawing.Imaging.Tests Assert.Throws<ExternalException>(() => new Metafile(GetPath(BmpFile))); } - [ActiveIssue(22598)] [ActiveIssue(20884, TestPlatforms.AnyUnix)] [ConditionalFact(Helpers.GdiplusIsAvailable)] public void Ctor_NullString_ThrowsArgumentNullException() @@ -93,13 +92,13 @@ namespace System.Drawing.Imaging.Tests yield return new object[] { string.Empty }; } - [ActiveIssue(22640)] [ActiveIssue(20884, TestPlatforms.AnyUnix)] [ConditionalTheory(Helpers.GdiplusIsAvailable)] - [MemberData(nameof(InvalidPath_TestData))] + [InlineData(@"fileNo*-//\\#@(found")] + [InlineData("")] public void Ctor_InvalidPath_ThrowsArgumentException(string path) { - AssertExtensions.Throws<ArgumentException>(null, () => new Metafile(path)); + AssertExtensions.Throws<ArgumentException>("path", null, () => new Metafile(path)); } [ActiveIssue(20884, TestPlatforms.AnyUnix)] @@ -113,12 +112,11 @@ namespace System.Drawing.Imaging.Tests } } - [ActiveIssue(22741)] [ActiveIssue(20884, TestPlatforms.AnyUnix)] [ConditionalFact(Helpers.GdiplusIsAvailable)] public void Ctor_NullStream_ThrowsArgumentException() { - AssertExtensions.Throws<ArgumentException>(null, () => new Metafile((Stream)null)); + AssertExtensions.Throws<ArgumentNullException, ArgumentException>("stream", null, () => new Metafile((Stream)null)); } [ActiveIssue(20884, TestPlatforms.AnyUnix)] @@ -414,7 +412,7 @@ namespace System.Drawing.Imaging.Tests File.Delete(fileName); } - + [ActiveIssue(20884, TestPlatforms.AnyUnix)] [ConditionalTheory(Helpers.GdiplusIsAvailable)] [MemberData(nameof(Description_TestData))] @@ -458,7 +456,6 @@ namespace System.Drawing.Imaging.Tests } } - [ActiveIssue(22723)] [ActiveIssue(20884, TestPlatforms.AnyUnix)] [ConditionalFact(Helpers.GdiplusIsAvailable)] public void Ctor_NullPath_ThrowsArgumentNullException() @@ -472,7 +469,6 @@ namespace System.Drawing.Imaging.Tests } } - [ActiveIssue(22723)] [ActiveIssue(20884, TestPlatforms.AnyUnix)] [ConditionalTheory(Helpers.GdiplusIsAvailable)] [InlineData(@"fileNo*-//\\#@(found")] @@ -482,18 +478,17 @@ namespace System.Drawing.Imaging.Tests using (var bufferGraphics = Graphics.FromHwndInternal(IntPtr.Zero)) { IntPtr referenceHdc = bufferGraphics.GetHdc(); - AssertExtensions.Throws<ArgumentException>(null, () => new Metafile(fileName, referenceHdc)); - AssertExtensions.Throws<ArgumentException>(null, () => new Metafile(fileName, referenceHdc, EmfType.EmfOnly)); - AssertExtensions.Throws<ArgumentException>(null, () => new Metafile(fileName, referenceHdc, EmfType.EmfOnly, "description")); + AssertExtensions.Throws<ArgumentException>("path", null, () => new Metafile(fileName, referenceHdc)); + AssertExtensions.Throws<ArgumentException>("path", null, () => new Metafile(fileName, referenceHdc, EmfType.EmfOnly)); + AssertExtensions.Throws<ArgumentException>("path", null, () => new Metafile(fileName, referenceHdc, EmfType.EmfOnly, "description")); } } - [ActiveIssue(22723)] [ActiveIssue(20884, TestPlatforms.AnyUnix)] [ConditionalFact(Helpers.GdiplusIsAvailable)] public void Ctor_PathTooLong_ThrowsPathTooLongException() { - string fileName = GetPath(new string('a', 261)); + string fileName = GetPath(new string('a', short.MaxValue)); using (var bufferGraphics = Graphics.FromHwndInternal(IntPtr.Zero)) { IntPtr referenceHdc = bufferGraphics.GetHdc(); @@ -955,18 +950,9 @@ namespace System.Drawing.Imaging.Tests [InlineData("")] public void Static_GetMetafileHeader_InvalidPath_ThrowsArgumentException(string fileName) { - AssertExtensions.Throws<ArgumentException>(null, () => Metafile.GetMetafileHeader(fileName)); - } - - [ActiveIssue(22747)] - [ActiveIssue(20884, TestPlatforms.AnyUnix)] - [ConditionalFact(Helpers.GdiplusIsAvailable)] - public void Static_GetMetafileHeader_PathTooLong_ThrowsPathTooLongException() - { - Assert.Throws<PathTooLongException>(() => Metafile.GetMetafileHeader(GetPath(new string('a', 261)))); + AssertExtensions.Throws<ArgumentException>("path", null, () => Metafile.GetMetafileHeader(fileName)); } - [ActiveIssue(22747)] [ActiveIssue(20884, TestPlatforms.AnyUnix)] [ConditionalFact(Helpers.GdiplusIsAvailable)] public void Static_GetMetafileHeader_NullString_ThrowsArgumentNullException() @@ -1090,7 +1076,7 @@ namespace System.Drawing.Imaging.Tests GraphicsUnit graphicsUnit = (GraphicsUnit)int.MaxValue; AssertMetafileHeaderIsBlank(metafile.GetMetafileHeader()); - Assert.Equal(new Rectangle(0, 0, 1, 1), metafile.GetBounds(ref graphicsUnit)); + Assert.Equal(new Rectangle(0, 0, 1, 1), metafile.GetBounds(ref graphicsUnit)); Assert.Equal(GraphicsUnit.Pixel, graphicsUnit); } |