diff --git a/public/Get-DbaBackupInformation.ps1 b/public/Get-DbaBackupInformation.ps1 index 828c7120351..4b6072d8d6f 100644 --- a/public/Get-DbaBackupInformation.ps1 +++ b/public/Get-DbaBackupInformation.ps1 @@ -326,6 +326,10 @@ function Get-DbaBackupInformation { $Files += Get-XpDirTreeRestoreFile -Path "$f$($separator)FULL" -SqlInstance $server -NoRecurse $Files += Get-XpDirTreeRestoreFile -Path "$f$($separator)DIFF" -SqlInstance $server -NoRecurse $Files += Get-XpDirTreeRestoreFile -Path "$f$($separator)LOG" -SqlInstance $server -NoRecurse + } elseif ($f -match "^s3://" -and [System.IO.Path]::GetExtension("$f").Length -eq 0) { + # An S3 folder: T-SQL cannot list S3 objects (see Get-XpDirTreeRestoreFile), and handing the + # folder to Read-DbaBackupHeader would only get it rejected for not being a file. + Stop-Function -Message "S3 paths cannot be enumerated using T-SQL. Use explicit file paths or PowerShell-based enumeration for S3 storage." -Target $f -Continue } else { Write-Message -Level VeryVerbose -Message "File" $Files += $f @@ -349,7 +353,17 @@ function Get-DbaBackupInformation { try { $FileDetails = Read-DbaBackupHeader -SqlInstance $server -Path $Files -StorageCredential $StorageCredential -EnableException } catch { - Stop-Function -Message "Failure on $($server.Name)" -ErrorRecord $PSItem -Target $server.Name -Continue + # This gives up on the paths of this process invocation, not on the command: a plain + # Stop-Function sets the interrupt flag that Test-FunctionInterrupt reads at the top of this + # block, which would drop every path piped in after these, and -Continue has no loop to + # continue here (#10638). So throw under -EnableException, otherwise warn, and return from + # this process invocation only. + if ($EnableException) { + Stop-Function -Message "Failure on $($server.Name)" -ErrorRecord $PSItem -Target $server.Name -EnableException $true + } else { + Write-Message -Level Warning -Message "Failure on $($server.Name)" -ErrorRecord $PSItem -Target $server.Name + } + return } } diff --git a/public/Read-DbaBackupHeader.ps1 b/public/Read-DbaBackupHeader.ps1 index 237e9225021..13bd6f9aae7 100644 --- a/public/Read-DbaBackupHeader.ps1 +++ b/public/Read-DbaBackupHeader.ps1 @@ -205,7 +205,7 @@ function Read-DbaBackupHeader { foreach ($p in $Path) { Write-Message -Level Verbose -Message "Checking: $p" if ([System.IO.Path]::GetExtension("$p").Length -eq 0) { - Stop-Function -Message "Path ("$p") should be a file, not a folder" -Category InvalidArgument + Stop-Function -Message "Path ($p) should be a file, not a folder" -Category InvalidArgument return } } diff --git a/tests/Get-DbaBackupInformation.Tests.ps1 b/tests/Get-DbaBackupInformation.Tests.ps1 index 4eba7ab549f..907fe20742c 100644 --- a/tests/Get-DbaBackupInformation.Tests.ps1 +++ b/tests/Get-DbaBackupInformation.Tests.ps1 @@ -267,4 +267,73 @@ Describe $CommandName -Tag IntegrationTests { $resultsSanLog.Count | Should -BeExactly 3 } } -} \ No newline at end of file + + Context "When a file is not a backup" { + BeforeAll { + # A text file where the instance can read it, so the header read fails on the content, not on access. + $notABackup = Join-Path -Path $TestConfig.Temp -ChildPath "dbatoolsci_notabackup_$(Get-Random).txt" + Set-Content -Path $notABackup -Value "dbatoolsci" + } + + AfterAll { + Remove-Item -Path $notABackup -Force -ErrorAction SilentlyContinue + } + + It "Warns without eating an iteration of the caller's loop" { + # The header-read catch used to run Stop-Function -Continue in the process block, where no loop + # encloses it - the continue escaped the command and consumed an iteration of this very loop, so + # the counter stayed at zero (#10638). + $loopCount = 0 + foreach ($i in 1..3) { + $null = Get-DbaBackupInformation -SqlInstance $TestConfig.InstanceSingle -Path $notABackup -WarningAction SilentlyContinue + $loopCount++ + } + $loopCount | Should -Be 3 + ($WarnVar -join " ") | Should -BeLike "*Failure on*" + # The last call carried the message once, not twice. + @($WarnVar | Where-Object { $PSItem -like "*Failure on*" }).Count | Should -Be 1 + } + + It "Still reads the backup piped in after the file that is not one" { + # A plain Stop-Function used to set the command-wide interrupt flag for the unreadable file, and + # Test-FunctionInterrupt then dropped every path piped in after it. + $validBackup = (Get-ChildItem -Path $DestBackupDir -Filter "$dbname*.bak" | Select-Object -First 1).FullName + $results = $notABackup, $validBackup | Get-DbaBackupInformation -SqlInstance $TestConfig.InstanceSingle -WarningAction SilentlyContinue + # The full and the differential of the fixture can share one file, so the file may hold two backup sets. + ($results | Measure-Object).Count | Should -BeGreaterThan 0 + $results.Database | Select-Object -Unique | Should -Be $dbname + ($WarnVar -join " ") | Should -BeLike "*Failure on*" + } + + It "Throws for the file that is not a backup under -EnableException" { + $headerException = $null + $headerWarnings = $null + try { + Get-DbaBackupInformation -SqlInstance $TestConfig.InstanceSingle -Path $notABackup -EnableException -WarningVariable headerWarnings + } catch { + $headerException = $PSItem + } + # Stop-Function rethrows the inner RESTORE HEADERONLY error, so the exception text is the SQL error, not the message. + $headerException | Should -Not -BeNullOrEmpty + # Stop-Function writes the message once before it throws, as it does for every throw in dbatools; it is not + # written a second time. + @($headerWarnings | Where-Object { $PSItem -like "*Failure on*" }).Count | Should -Be 1 + } + } + + Context "When Path is an S3 folder" { + It "Warns that S3 folders cannot be enumerated and returns nothing" { + # An S3 URL skips xp_dirtree, and a folder then went to Read-DbaBackupHeader as if it were a file, which + # rejected it with a message about files and folders instead of the one about S3 enumeration. On top of + # that the escaping continue ended the CI test before its assertions, so nobody noticed. + $splatS3Folder = @{ + SqlInstance = $TestConfig.InstanceSingle + Path = "s3://dbatoolsci.invalid/bucket/folder/" + WarningAction = "SilentlyContinue" + } + $results = Get-DbaBackupInformation @splatS3Folder + $results | Should -BeNullOrEmpty + ($WarnVar -join " ") | Should -BeLike "*S3 paths cannot be enumerated using T-SQL*" + } + } +} diff --git a/tests/Read-DbaBackupHeader.Tests.ps1 b/tests/Read-DbaBackupHeader.Tests.ps1 index 6f2853d5512..f9ccdc6a922 100644 --- a/tests/Read-DbaBackupHeader.Tests.ps1 +++ b/tests/Read-DbaBackupHeader.Tests.ps1 @@ -27,4 +27,21 @@ Describe $CommandName -Tag UnitTests { Integration test should appear below and are custom to the command you are writing. Read https://github.com/dataplat/dbatools/blob/development/contributing.md#tests for more guidence. -#> \ No newline at end of file +#> + +Describe $CommandName -Tag IntegrationTests { + Context "When Path is a folder" { + It "Warns that it needs a file and returns nothing" { + # The message used to be built from a broken string literal, "Path ("$p") should be a file, not a folder", + # which handed $p to Stop-Function as a positional argument and turned the warning into a binding error. + $splatFolder = @{ + SqlInstance = $TestConfig.InstanceSingle + Path = $TestConfig.Temp + WarningAction = "SilentlyContinue" + } + $results = Read-DbaBackupHeader @splatFolder + $results | Should -BeNullOrEmpty + ($WarnVar -join " ") | Should -BeLike "*should be a file, not a folder*" + } + } +}