Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Conversation

@Anipik
Copy link

@Anipik Anipik commented Mar 7, 2018

Fixes #25755

@Anipik Anipik requested review from JeremyKuhne and danmoseley March 7, 2018 06:14
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)
Copy link
Member

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)

Copy link
Author

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.

Copy link
Member

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));
Copy link
Member

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()
Copy link
Member

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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ThrowsException

Copy link
Member

@danmoseley danmoseley left a 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.

@danmoseley
Copy link
Member

@dotnet-bot test NETFX x86 Release Build please (fixed hang)

@danmoseley
Copy link
Member

@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
Copy link
Member

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()));
Copy link
Member

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

@Anipik Anipik merged commit 59f6987 into dotnet:master Mar 8, 2018
caesar-chen pushed a commit to caesar-chen/corefx that referenced this pull request Mar 8, 2018
@karelz karelz added this to the 2.1.0 milestone Mar 10, 2018
@Anipik Anipik deleted the CreateSub branch March 24, 2018 01:33
JeremyKuhne added a commit to JeremyKuhne/corefx that referenced this pull request Jun 11, 2018
This broke with dotnet#27810. Reworked the logic in an attempt to be clearer and added test cases.

This addresses #30283.
danmoseley pushed a commit that referenced this pull request Jun 13, 2018
#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
JeremyKuhne added a commit to JeremyKuhne/corefx that referenced this pull request Jun 13, 2018
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
daxian-dbw pushed a commit to daxian-dbw/PowerShell that referenced this pull request Jun 13, 2018
…o.CreateSubDirectory

Change to use Directory.CreateDirectory because of regression introduced by dotnet/corefx#27810 in creating a subdirectory under root path.
JeremyKuhne added a commit that referenced this pull request Jun 27, 2018
#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
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
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
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants