Powershell: Violation of the (great) separation of concerns design principle in Join-Path

Created on 29 Jul 2017  路  14Comments  路  Source: PowerShell/PowerShell

Good evening

it's really hard to complain this issue because it's a discussion about the most basic design principles we know and love snce many years:

Functions (in PowerShell commands) should never do things which the user does not expect.
Such things are well known as side-effects and we had to fight with them 20 years ago as Spaghetti code was widely used.

Therefore it hurts to see that those Problems are comming back.

The concrete problem:
Join-Path does no longer solve the problem which we expect (join path items),
but it also checks if a drive exists.

The essence behind this "logic" is that _no one is joining path items with non existing drive letters_. Of course, this is wrong.
But to get a consitent logic, the next version of Join-Path must automatically create any non-exsting path - it's the same logic: _no one is joining path items with a non existing path_.

Beside the inconsistent logic, we have more very annoying side effects:

  • Existing Code no longer works
  • Users are searching for functions which are solvong the problem to join path items
  • If someone is writing a function which is just joining path items - how should it get called?
    Join-Path-Without-Test?
    maybe Just-Join-Path?
    Of course, it must be called Join-Path.

There are many better options to handle the Problem, for example:

  • Add a Parameter (e.g. -AssertDrive and -EnforceDir), but don't change the default logic because it is already perfect
  • Create another function, e.g. Join-Path-Assert
  • Pass the result of Join-Path to a function which is testing if the drive exists

Kind regards,
Thomas

Steps to reproduce

# If the Drive Q: does not exists:
PS C:\Users\schittli> Join-Path 'q:\' 'test'
Join-Path : Cannot find drive. A drive with the name 'q' does not exist.

Expected behavior

Join-Path ist doing what we expect: join path Elements. Nothing else.

Actual behavior

Join-Path has an enforced side-effect and it does break the separation of concerns design principle

Environment data

> $PSVersionTable
Name                           Value
----                           -----
PSVersion                      5.1.14393.1480
PSEdition                      Desktop
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0...}
BuildVersion                   10.0.14393.1480
CLRVersion                     4.0.30319.42000
WSManStackVersion              3.0
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1
Area-Cmdlets-Management Committee-Reviewed Issue-Discussion

Most helpful comment

@PowerShell/powershell-committee reviewed this. Expectation is that if given a drive that exists, it should use that provider's path separator. If given a drive does not exist, then it will default to the current provider's path separator. An additional -Provider parameter may be added to allow specifying an explicit provider (this should be a separate issue).

All 14 comments

Would a -force be an adequate solution?

@SteveL-MSFT could you explain in detail?

Do you mean that you would now need to use Join-Path -Force if your drive doesn't exist?
Do you mean that Join-Path -Force would do the drive check, and you would restore the original (much better) behavior?

Because the first option sucks; this is a pretty big breaking change that most affects the people who were trying to do the "Right Thing" by not hardcoding \, with the goal of supporting future platforms.

Besides that aspect, it's not at all intuitive that a function that joins paths should check whether part of it actually exists.

The .Net framework for example didn't change this to check for existence: [System.IO.Path]::Combine('Q:','One','Two')

I really hope this gets restored.

When did this behavior change? Trying to determine how impactful such a breaking change would be where we either default to checking or default to not checking.

For both PowerShell 6.0.2 and in Windows Powershell (tested 5.1.14409.1005, 5.1.16299.251, and 5.1.17134.1)

This does work:

  • Join-Path -Path 'DOMAIN' -ChildPath 'John'
  • Join-Path -Path '\\dummy\resource' -ChildPath 'Folder'
  • Split-Path -Path 'DOMAIN\John' -Leaf
  • Split-Path -Path 'Y:\Folder' -Leaf
  • Split-Path -Path '\\dummy\resource\Folder' -Leaf

This does not work:

  • Join-Path -Path 'Y:\' -ChildPath 'Folder'

    • Error: Join-Path : Cannot find drive. A drive with the name 'Y' does not exist.

Since this is in both editions (correct me if I'm wrong) there would be a breaking change if Join-Path suddenly didn't throw an error. So a parameter to override that behavior could be added which would tell Join-Path not to validate a rooted path. User could then instead accomplished that with Resolve-Path.

Note: Didn't have a Windows PowerShell 5.1.14393 to test with.

@johlju thanks for the data. However, some of the discussion above implies that this behavior of checking if the path is valid is a regression from previous behavior and I'm trying to understand which version of PowerShell had the different behavior. Trying to understand if the discussion is putting behavior back to what it was or requesting a breaking change. This is why I suggested above to perhaps use -Force (or perhaps more specifically something like -NoValidate) to enable the new desired behavior without a breaking change.

It may be a breaking change... but I think this is a bug..,, and I don't think fixing broken behavior as a result of a bug is really a breaking change

https://github.com/PowerShell/PowerShell/blob/master/docs/dev-process/breaking-change-contract.md#acceptable-changes

Any existing behavior that results in an error message generally may be changed to provide new functionality.

This is the relevant code:

https://github.com/PowerShell/PowerShell/blob/80951777cfde34e9d527dedd833f90cc715ad684/src/System.Management.Automation/engine/SessionStateNavigation.cs#L935-L942

Globber.GetProviderPath() is where the exception is thrown. But, after where the exception is thrown, there is logic to check for a null drive. This along with the comment about ignoring the result indicates to me that this should be wrapped in a try/catch. If I'm interpreting this right, it seems the intent was to just use the GetProvderPath() to try to get the provider and drive if possible, If not, it falls back to the current provider and drive.

I propose that Globber.GetProviderPath() be wrapped in a throwaway try/catch

Sadly, I tried this and a few other things and it turns out I was wrong. 鈽癸笍
This would require some more in-depth debugging.

As far as I understand, the problem is because PowerShell does not know how to join paths. It need to redirect that operation to target provider. But if you specify non-existent drive, then which provider should it use? For example Alias, Environment, Function and Variable providers does not support navigation and produce different result from what it would for FileSystem provider:

PS> Join-Path alias:\asd fgh
Alias:\fgh
PS> Join-Path env:\asd fgh
Env:\fgh
PS> Join-Path function:\asd fgh
Function:\fgh
PS> Join-Path variable:\asd fgh
Variable:\fgh

cc @joeyaiello

This came up in documentation discussions, I would naively propose that, if the PSDrive can't be resolved, that we fall back to default filesystem path separators for the given platform.

This would fall in line with what we do with non-drive Join-Path calls:

join-path 'foo' 'bar'
foo\bar

Thoughts?

I think this is a decent assumption that the filesystem is the target for a non existent drive.

Just as an alternative solution to consider: Using the current location's provider.

@markekraus's suggestion to use the current location provider makes sense to me

I will add that my proposal is how join-path works today.

Set-Location AD:\
join-path 'foo' 'bar'

result:

bar,foo

Since the ActiveDirectory provider uses , for the path separator.

@PowerShell/powershell-committee reviewed this. Expectation is that if given a drive that exists, it should use that provider's path separator. If given a drive does not exist, then it will default to the current provider's path separator. An additional -Provider parameter may be added to allow specifying an explicit provider (this should be a separate issue).

Was this page helpful?
0 / 5 - 0 ratings