-
Notifications
You must be signed in to change notification settings - Fork 4.9k
WhiteSpace and PrefixParent subdirectory names throw exception #27810
Conversation
| string fullPath = Path.GetFullPath(Path.Combine(FullPath, path)); | ||
|
|
||
| if (0 != string.Compare(FullPath, 0, fullPath, 0, FullPath.Length, PathInternal.StringComparison)) | ||
| if (0 != string.Compare(FullPath, 0, fullPath, 0, FullPath.Length, PathInternal.StringComparison) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If yo'ure changing this you can fix the Yoda conditional. Better still, change to string.Equals instead. It can be faster (eg if strings are different lengths it bails immediately)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
String.Equals does not have any overload to match a string with a substring of second string.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I missed that, you're right. Maybe there should be a STring.Equals(Span, Span)...
| public void ParentDirectoryNameAsPrefix() | ||
| { | ||
| string randomName = GetTestFileName(); | ||
| var di = new DirectoryInfo(Path.Combine(TestDirectory, randomName)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
simpler to write Directory.CreateDirectory(Path.Combine(TestDirectory, randomName))?
|
|
||
| [Fact] | ||
| [SkipOnTargetFramework(TargetFrameworkMonikers.NetFramework)] | ||
| public void ParentDirectoryNameAsPrefix() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ParentDirectoryNameAsPrefixShouldThrow ?
| MemberData(nameof(SimpleWhiteSpace))] | ||
| [PlatformSpecific(TestPlatforms.Windows)] // Simple whitespace is trimmed in path | ||
| [SkipOnTargetFramework(TargetFrameworkMonikers.NetFramework)] | ||
| public void WindowsSimpleWhiteSpaceThrowException(string component) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ThrowsException
danmoseley
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Jeremy should signoff as I dno't have full context.
|
@dotnet-bot test NETFX x86 Release Build please (fixed hang) |
|
@Anipik the Linux failures are real. |
|
|
||
| if (0 != string.Compare(FullPath, 0, fullPath, 0, FullPath.Length, PathInternal.StringComparison)) | ||
| if (string.Compare(FullPath, 0, fullPath, 0, FullPath.Length, PathInternal.StringComparison) != 0 | ||
| || fullPath.Length < FullPath.Length |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do the faster checks first. string.Compare should be last.
| string randomName = GetTestFileName(); | ||
| DirectoryInfo di = Directory.CreateDirectory(Path.Combine(TestDirectory, randomName)); | ||
|
|
||
| Assert.Throws<ArgumentException>(() => di.CreateSubdirectory(@".." + Path.DirectorySeparatorChar + randomName + @"abc" + Path.DirectorySeparatorChar + GetTestFileName())); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can use Path.Combine here for clarity
This broke with dotnet#27810. Reworked the logic in an attempt to be clearer and added test cases. This addresses #30283.
#30293) * Fix creating subdirectories under directories with trailing separators This broke with #27810. Reworked the logic in an attempt to be clearer and added test cases. This addresses #30283. * Flip the logic to make it a little easier to follow. (Reduces the number of nots.) * Skip new test on desktop
dotnet#30293) * Fix creating subdirectories under directories with trailing separators This broke with dotnet#27810. Reworked the logic in an attempt to be clearer and added test cases. This addresses #30283. * Flip the logic to make it a little easier to follow. (Reduces the number of nots.) * Skip new test on desktop
…o.CreateSubDirectory Change to use Directory.CreateDirectory because of regression introduced by dotnet/corefx#27810 in creating a subdirectory under root path.
#30293) * Fix creating subdirectories under directories with trailing separators This broke with #27810. Reworked the logic in an attempt to be clearer and added test cases. This addresses #30283. * Flip the logic to make it a little easier to follow. (Reduces the number of nots.) * Skip new test on desktop
dotnet/corefx#30293) * Fix creating subdirectories under directories with trailing separators This broke with dotnet/corefx#27810. Reworked the logic in an attempt to be clearer and added test cases. This addresses dotnet/corefx#30283. * Flip the logic to make it a little easier to follow. (Reduces the number of nots.) * Skip new test on desktop Commit migrated from dotnet/corefx@4a6f54e
Fixes #25755