Fix Start-Transcript input path validation (#1749)

src/Microsoft.PowerShell.ConsoleHost/host/msh/StartTranscriptCmdlet.cs

@@ -215,9 +215,11 @@ namespace Microsoft.PowerShell.Commands
                         }
                         else
                         {
-                            Exception innerException = PSTraceSource.NewArgumentException(effectiveFilePath,
+                            string errorMessage = String.Format(
+                                System.Globalization.CultureInfo.InvariantCulture,
                                 TranscriptStrings.TranscriptFileReadOnly,
                                 effectiveFilePath);
+                            Exception innerException = new ArgumentException(errorMessage);
                             ThrowTerminatingError(new ErrorRecord(innerException, "FileReadOnly", ErrorCategory.InvalidArgument, effectiveFilePath));
                         }
                     }

src/System.Management.Automation/engine/hostifaces/MshHostUserInterface.cs

@@ -670,6 +670,23 @@ namespace System.Management.Automation.Host
                         }
                     }
 
+                    // Create the file in the main thread and flush the contents in the background thread.
+                    // Transcription should begin only if file generation is successful.
+                    // If there is an error in file generation, throw the exception.
+                    string baseDirectory = Path.GetDirectoryName(transcript.Path);
+                    if (Directory.Exists(transcript.Path) || (String.Equals(baseDirectory, transcript.Path.TrimEnd(Path.DirectorySeparatorChar), StringComparison.Ordinal)))
+                    {
+                        throw new ArgumentException(InternalHostUserInterfaceStrings.InvalidTranscriptFilePath);
+                    }
+                    if(!Directory.Exists(baseDirectory))
+                    {
+                        Directory.CreateDirectory(baseDirectory);   
+                    }
+                    if(!File.Exists(transcript.Path))
+                    {
+                        File.Create(transcript.Path).Dispose();
+                    }
+                    
                     // Do the actual writing in the background so that it doesn't hold up the UI thread.
                     Task writer = Task.Run(() =>
                     {
@@ -682,12 +699,6 @@ namespace System.Management.Automation.Host
                         {
                             try
                             {
-                                string baseDirectory = Path.GetDirectoryName(transcript.Path);
-                                if (!Directory.Exists(baseDirectory))
-                                {
-                                    Directory.CreateDirectory(baseDirectory);
-                                }
-
                                 transcript.FlushContentToDisk();
                                 written = true;
                             }

src/System.Management.Automation/resources/InternalHostUserInterfaceStrings.resx

@@ -213,4 +213,7 @@ Windows PowerShell transcript end
 End time: {0:yyyyMMddHHmmss}
 **********************</value>
   </data>
+   <data name="InvalidTranscriptFilePath" xml:space="preserve">
+    <value>Cannot perform operation because file path resolves to a directory.</value>
+  </data>
 </root>

test/powershell/Modules/Microsoft.Powershell.Host/Start-Transcript.Tests.ps1

@@ -0,0 +1,99 @@
+Describe "Start-Transcript, Stop-Transcript tests" -tags "CI" {
+    
+    BeforeAll {
+
+        function ValidateTranscription {
+            param
+            (
+                [string] $scriptToExecute,
+                [string] $outputFilePath,
+                [switch] $append,
+                [switch] $noClobber,
+                [string] $expectedError
+            )
+            if($append -or $noClobber)
+            {
+                #Add sample text to the file
+                $content = "This is sample text!"
+                $content | Out-File -FilePath $outputFilePath
+                Test-Path $outputFilePath | Should be $true
+            }
+            #execute script
+            $ps = [powershell]::Create()
+            $ps.addscript($scriptToExecute).Invoke()
+            $ps.commands.clear()
+
+            if($expectedError)
+            {
+                $ps.hadErrors | Should be $true
+                $ps.Streams.Error.FullyQualifiedErrorId | Should be $expectedError
+            }
+            else 
+            {
+                $ps.addscript("Get-Date").Invoke()
+                $ps.commands.clear()
+                $ps.addscript("Stop-Transcript").Invoke()
+
+                Test-Path $outputFilePath | Should be $true
+                $outputFilePath | should contain "Get-Date"
+                if($append)
+                {
+                    $outputFilePath | Should contain $content
+                }
+            }
+        }
+        ## function ends here
+
+        $transcriptFilePath = join-path $TestDrive "transcriptdata.txt"
+        Remove-Item $transcriptFilePath -Force -ErrorAction SilentlyContinue   
+    }
+    
+
+    AfterEach {
+        Remove-Item $transcriptFilePath -ErrorAction SilentlyContinue
+    }
+
+    It "Should create Transcript file at default path" {
+        $script = "Start-Transcript"
+        if ($isWindows)
+        {
+            $defaultTranscriptFilePath = [io.path]::Combine($env:USERPROFILE, "Documents", "PowerShell_transcript*")
+        }
+        else 
+        {
+            $defaultTranscriptFilePath = [io.path]::Combine($env:HOME, "PowerShell_transcript*")
+        }
+        #Make sure there is no stale data
+        Remove-Item $defaultTranscriptFilePath -Force -ErrorAction SilentlyContinue
+        ValidateTranscription -scriptToExecute $script -outputFilePath $defaultTranscriptFilePath
+        #Remove test data
+        Remove-Item $defaultTranscriptFilePath -ErrorAction SilentlyContinue
+    }
+    It "Should create Transcript file with 'Path' parameter" {
+        $script = "Start-Transcript -path $transcriptFilePath"
+        ValidateTranscription -scriptToExecute $script -outputFilePath $transcriptFilePath 
+    }
+    It "Should create Transcript file with 'LiteralPath' parameter" {
+        $script = "Start-Transcript -LiteralPath $transcriptFilePath"
+        ValidateTranscription -scriptToExecute $script -outputFilePath $transcriptFilePath
+    }
+    It "Should create Transcript file with 'OutputDirectory' parameter" {
+        $script = "Start-Transcript -OutputDirectory $TestDrive"
+        $outputFilePath = join-path $TestDrive "PowerShell_transcript*"
+        ValidateTranscription -scriptToExecute $script -outputFilePath $outputFilePath
+    }
+    It "Should Append Transcript data in existing file if 'Append' parameter is used with Path parameter" {
+        $script = "Start-Transcript -path $transcriptFilePath -Append"
+        ValidateTranscription -scriptToExecute $script -outputFilePath $transcriptFilePath -append
+    }
+    It "Should return an error if the file exists and NoClobber is used" {
+        $script = "Start-Transcript -path $transcriptFilePath -NoClobber"
+        $expectedError = "NoClobber,Microsoft.PowerShell.Commands.StartTranscriptCommand"
+        ValidateTranscription -scriptToExecute $script -outputFilePath $transcriptFilePath -noClobber -expectedError $expectedError
+    }
+    It "Should return an error if the path resolves to an existing directory" {
+        $script = "Start-Transcript -path $TestDrive"
+        $expectedError = "CannotStartTranscription,Microsoft.PowerShell.Commands.StartTranscriptCommand"
+        ValidateTranscription -scriptToExecute $script -outputFilePath $null -expectedError $expectedError
+    }
+}