This repository was archived by the owner on Jan 23, 2023. It is now read-only.
Fix creating subdirectories under directories with trailing separators #30293
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This broke with dotnet#27810. Reworked the logic in an attempt to be clearer and added test cases. This addresses #30283.
pjanotti
approved these changes
Jun 12, 2018
Contributor
pjanotti
left a comment
There was a problem hiding this comment.
I guess that there are other tests that already cover the case when there is no directory separator on the end of the path. LGTM.
Anipik
approved these changes
Jun 12, 2018
danmoseley
reviewed
Jun 12, 2018
| ReadOnlySpan<char> trimmedCurrentPath = PathInternal.TrimEndingDirectorySeparator(FullPath.AsSpan()); | ||
|
|
||
| // We want to make sure the requested directory is actually under the subdirectory. | ||
| if (!trimmedNewPath.StartsWith(trimmedCurrentPath, PathInternal.StringComparison) |
Member
There was a problem hiding this comment.
I found it easier to grok if I flip it around, up to you
// Make sure the requested directory is actually under the subdirectory.
if (trimmedNewPath.StartsWith(trimmedCurrentPath, PathInternal.StringComparison)
{
// Note we must also check the first character after the current path is a slash.
// Otherwise passing "c:\foo" and "..\foobar", which would produce a new path of "c:\foobar", would be allowed.
if ((trimmedNewPath.Length == trimmedCurrentPath.Length) || PathInternal.IsDirectorySeparator(newPath[trimmedCurrentPath.Length])
{
FileSystem.CreateDirectory(newPath);
return new DirectoryInfo(newPath);
}
}
throw new ArgumentException(SR.Format(SR.Argument_InvalidSubPath, path, FullPath), nameof(path));
Member
Author
There was a problem hiding this comment.
I found it easier to grok if I flip it around
I agree- it reduces the negatives. Updating.
danmoseley
approved these changes
Jun 12, 2018
(Reduces the number of nots.)
Member
|
@JeremyKuhne it's not super obvious (https://github.com/dotnet/core-eng/issues/3107) but your test failure is in NETFX. The new test is not failing on NETFX as expected. Perhaps that means we need to be more lenient in .NET Core. |
Member
|
Cross check leg is optional right now. |
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
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
Member
|
This PR was fixed in master - hence 3.0. The change was ported into release/2.1 branch in PR #30367. |
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.
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This broke with #27810. Reworked the logic in an attempt to be clearer and added test cases.
This addresses #30283.