Fix added because somehow the $server variable contained a space...

[nathanhere]

between the server name and port, which would cause conn.Open to hang and terminate with an error.

[tresf]

@nathanhere thanks. Can you explain why the space was there to begin with? Is this value being provided by another application?

I assume you're trying to handle the edge-case that...

$server = "foo 1433"
# instead of... 
$server = "foo,1433"

[tresf]

Note, this will also have the side-effect of putting commas at the end of strings...

$server = "foo  "
# would become... 
$server = "foo,,"

[nathanhere]

Happy to provide a bit more detail to the environment I used to run this (Note the caveat that specifying port 1433 isn't required for this script, but it's my practice due to some cli commands requiring it explicitly, like sqlcmd).

In a MacOS I'm using a Windows port of Powershell within the terminal, which connects to a MS SQL Server 2017 Docker container. I found that when I ran the command:

./GetMsSqlDump.ps1 -server localhost,1433 -username SA -password somepassword -db testdb –table dbo.* –file dump.sql –overwrite –noidentity

I kept getting this error:

Exception calling "Open" with "0" argument(s): "A network-related or instance-specific error occurred while establishing a connection to SQL Server. The server was not found or was not accessible. Verify that the instance name is correct and that SQL Server is configured to allow remote connections. (provider: TCP Provider, error: 35 - An internal exception was caught)"
At ./GetMsSqlDump.ps1:278 char:1
+ $conn.Open()
+ ~~~~~~~~~~~~
+ CategoryInfo          : NotSpecified: (:) [], ParentContainsErrorRecordException
+ FullyQualifiedErrorId : SqlException

Prior to the main script body running, upon printing the variable $server , I found it contained localhost 1433, instead of localhost,1433, which was really unexplainable, because my input was localhost,1433, and there were no explicit operations within the script that touched the $server variable at that point. Also found the same phenomenon happening when printing the $connStr variable, which uses the $server variable. I can only currently hypothesize that the Windows port of Powershell to MacOS might possibly parse commas (,) within command arguments as spaces.

Due to the newness of this script to me (and powershell itself, especially powershell in a MacOS environment), I was unaware of an edge case (from user input) would cause $server to contain "foo ", but instead would be parsed by Powershell to "foo"--thus I assumed implementing replace() prior to the rest of the script execution was safe fix.

If you're able to replicate this on a MacOS with the Windows port of Powershell, let me know if you observe the same issue.

And thanks for providing this script!

[tresf]

I bet PS is automagically treating the comma as an array type, then the spaces are inserted when toString is invoked. How about we test for array instead? That would allow us to pop the elements off predictably without any magic space logic.

[tresf]

Yep... Check out this question...

https://stackoverflow.com/a/11990733/3196753

Let's assume others will make this same mistake and check for array. I think it'll be cleaner and more scalable.

[tresf]

Reproduced. Weird. Even stranger, $server -is [array] returns false, so there must be some silent conversion happening since the parameter is declared as a string type.

I'm fine with this PR so as long as we sanitize the end of the hostname using trim() or similar. This is important so that we don't go adding commas where they're not wanted or needed.

For example:

./GetMsSqlDump.ps1 -server "localhost   "
# ... would change to
./GetMsSqlDump.ps1 -server "localhost,,,"

I've tested the spaces after hostname, and the script works just fine, so trim() will ensure we don't add any regressions with this patch.

RFC 952 (and RFC 1123) says no symbols, punctuation characters, or whitespaces are permitted in a hostname. With that said, I would consider the proposed string replacement a sane (and in this case, our ONLY viable) technique.

So add the trim logic, a better code comment, and I'd be happy to merge.

For code comment, I'd recommend changing it to something like this:

# Handle unquoted $server translating commas to spaces