Powershell: Invoke-Command -SSHConnection breaks if the HostName string is [psobject]-wrapped

Created on 2 Oct 2019  路  7Comments  路  Source: PowerShell/PowerShell

The root cause is that a string returned from a cmdlet call or external program has an invisible extra [psobject] wrapper, and the Invoke-Command cmdlet fails to take that possibility into account.

In short: The command below breaks, because (hostname) has such an invisible wrapper, as evidenced by (hostname) -is [psobject] being $true.

This is yet another manifestation of our old friend #5579.

The specific problem in Invoke-Command is here.

Steps to reproduce

{ icm -SSHConnection @{ Hostname = (hostname) } -ScriptBlock { 'hi' }  } | 
  Should -not -Throw

Expected behavior

The test should succeed, if the local computer is set up for being a SSH remoting target.

Actual behavior

The test fails:

Expected no exception to be thrown, but an exception 
"The provided SSHConnection hashtable parameter name or element is null or empty." was thrown from line:1 char:3

Note that using (hostname).psobject.BaseObject or "$(hostname)" or (hostname).ToString() makes the problem go away.

As an aside: if the host name were truly invalid due not being a string, the error message would be misleading, because the problem is that the entry is of the _wrong type_, not that it is _null or empty_.

A further aside: The comment above the SSHConnection parameter definition is incomplete: keys Port and Subsystem are missing.

Environment data

PowerShell Core v7.0.0-preview.4
Area-Cmdlets-Core Hacktoberfest Issue-Bug Resolution-Fixed

Most helpful comment

@iSazonov Yeah I can grab it

All 7 comments

We have LanguagePrimitives.TryConvertTo. Question is should we accept only PSObject with base string type or we need to try convert any PSObject to string?

https://github.com/PowerShell/PowerShell/blob/950e4ba0820406a451c83fbc3ac75bf7cc840029/src/System.Management.Automation/engine/remoting/commands/PSRemotingCmdlet.cs#L1061-L1069

@iSazonov:

The current code accepts string only, and I think that's reasonable.

I don't think that users will gain anything if we support other types here - at least I don't see what other types might be useful in this scenario.

If we stick with the current approach, I recommend implementing amending the existing error message to say something like "element is null, empty, _or of the wrong type_".

I think it should follow the same path that a normal parameter binding would, so LanguagePrimitives.ConvertTo makes the most sense imo (probably don't need TryConvertTo, everything can be converted to a string in some way).

That makes sense, @SeeminglyScience - I expect the only real problem for end users to be the scenario detailed in this report.

However, the point about amending the error message still applies, because there's also GetSSHConnectionIntParameter for int values - where it's definitely possible to pass the wrong type.

Speaking of: The latter currently accepts an int value _only_; passing a long breaks, for instance.
Thus, I suggest modifying GetSSHConnectionIntParameter to use LanguagePrimitives.ConvertTo as well - see https://github.com/PowerShell/PowerShell/blob/950e4ba0820406a451c83fbc3ac75bf7cc840029/src/System.Management.Automation/engine/remoting/commands/PSRemotingCmdlet.cs#L1078

@SeeminglyScience Can you grab this? I already have over 15 opened PRs but I'd prefer to get the null reference exception fixed before GA.

@iSazonov Yeah I can grab it

:tada:This issue was addressed in #10720, which has now been successfully released as v7.0.0-preview.5.:tada:

Handy links:

Was this page helpful?
0 / 5 - 0 ratings