From 4f8f6c1fe57f43aea03ae38aea6c8aa395da3658 Mon Sep 17 00:00:00 2001 From: Xavier Hahn Date: Tue, 27 Oct 2020 22:29:42 +0100 Subject: [PATCH 01/10] Remove Encoding property in TranscriptionOption --- .../engine/hostifaces/MshHostUserInterface.cs | 23 +------------------ 1 file changed, 1 insertion(+), 22 deletions(-) diff --git a/src/System.Management.Automation/engine/hostifaces/MshHostUserInterface.cs b/src/System.Management.Automation/engine/hostifaces/MshHostUserInterface.cs index 09d4d9a9075..1f66a42003f 100644 --- a/src/System.Management.Automation/engine/hostifaces/MshHostUserInterface.cs +++ b/src/System.Management.Automation/engine/hostifaces/MshHostUserInterface.cs @@ -1068,22 +1068,7 @@ internal TranscriptionOption() /// /// The path that this transcript is being logged to. /// - internal string Path - { - get - { - return _path; - } - - set - { - _path = value; - // Get the encoding from the file, or default (UTF8-NoBom) - Encoding = Utils.GetEncoding(value); - } - } - - private string _path; + internal string Path { get; set; } /// /// Any output to log for this transcript. @@ -1101,12 +1086,6 @@ internal string Path /// internal bool IncludeInvocationHeader { get; set; } - /// - /// The encoding of this transcript, so that appending to it - /// can be done correctly. - /// - internal Encoding Encoding { get; private set; } - /// /// Logs buffered content to disk. We use this instead of File.AppendAllLines /// so that we don't need to pay seek penalties all the time, and so that we From ca6123d3092ce37374a77fd9e61b4a086a79be76 Mon Sep 17 00:00:00 2001 From: Xavier Hahn Date: Tue, 27 Oct 2020 22:47:07 +0100 Subject: [PATCH 02/10] Simplify Utils.GetEncoding to use StreamReader Utils.GetEncoding was using a complicated implementation that could be easily replaced by StreamReader.GetCurrentEncoding. Used that implementation in the FlushContentToDisk method --- .../engine/Utils.cs | 55 +------------------ .../engine/hostifaces/MshHostUserInterface.cs | 5 +- 2 files changed, 6 insertions(+), 54 deletions(-) diff --git a/src/System.Management.Automation/engine/Utils.cs b/src/System.Management.Automation/engine/Utils.cs index 0396f11c76a..5cac8729279 100644 --- a/src/System.Management.Automation/engine/Utils.cs +++ b/src/System.Management.Automation/engine/Utils.cs @@ -1399,66 +1399,17 @@ internal static Encoding GetEncoding(string path) return ClrFacade.GetDefaultEncoding(); } - byte[] initialBytes = new byte[100]; - int bytesRead = 0; - try { - using (FileStream stream = System.IO.File.OpenRead(path)) + using (StreamReader sr = new StreamReader(path, true)) { - using (BinaryReader reader = new BinaryReader(stream)) - { - bytesRead = reader.Read(initialBytes, 0, 100); - } + return sr.CurrentEncoding; } } - catch (IOException) + catch (System.Exception) { return ClrFacade.GetDefaultEncoding(); } - - // Test for four-byte preambles - string preamble = null; - Encoding foundEncoding = ClrFacade.GetDefaultEncoding(); - - if (bytesRead > 3) - { - preamble = string.Join("-", initialBytes[0], initialBytes[1], initialBytes[2], initialBytes[3]); - - if (encodingMap.TryGetValue(preamble, out foundEncoding)) - { - return foundEncoding; - } - } - - // Test for three-byte preambles - if (bytesRead > 2) - { - preamble = string.Join("-", initialBytes[0], initialBytes[1], initialBytes[2]); - if (encodingMap.TryGetValue(preamble, out foundEncoding)) - { - return foundEncoding; - } - } - - // Test for two-byte preambles - if (bytesRead > 1) - { - preamble = string.Join("-", initialBytes[0], initialBytes[1]); - if (encodingMap.TryGetValue(preamble, out foundEncoding)) - { - return foundEncoding; - } - } - - // Check for binary - string initialBytesAsAscii = System.Text.Encoding.ASCII.GetString(initialBytes, 0, bytesRead); - if (initialBytesAsAscii.IndexOfAny(nonPrintableCharacters) >= 0) - { - return Encoding.Unicode; - } - - return utf8NoBom; } // BigEndianUTF32 encoding is possible, but requires creation diff --git a/src/System.Management.Automation/engine/hostifaces/MshHostUserInterface.cs b/src/System.Management.Automation/engine/hostifaces/MshHostUserInterface.cs index 1f66a42003f..05d5819b6b6 100644 --- a/src/System.Management.Automation/engine/hostifaces/MshHostUserInterface.cs +++ b/src/System.Management.Automation/engine/hostifaces/MshHostUserInterface.cs @@ -1099,13 +1099,14 @@ internal void FlushContentToDisk() { if (_contentWriter == null) { + var encoding = Utils.GetEncoding(this.Path); try { // Try to first open the file with permissions that will allow us to read from it // later. _contentWriter = new StreamWriter( new FileStream(this.Path, FileMode.OpenOrCreate, FileAccess.ReadWrite, FileShare.Read), - this.Encoding); + encoding); _contentWriter.BaseStream.Seek(0, SeekOrigin.End); } catch (IOException) @@ -1114,7 +1115,7 @@ internal void FlushContentToDisk() // file permissions. _contentWriter = new StreamWriter( new FileStream(this.Path, FileMode.Append, FileAccess.Write, FileShare.Read), - this.Encoding); + encoding); } _contentWriter.AutoFlush = true; From a5849eae4c88b64bd1af405b5045b5a59ec989a1 Mon Sep 17 00:00:00 2001 From: Xavier Hahn Date: Tue, 27 Oct 2020 23:08:22 +0100 Subject: [PATCH 03/10] Catch a more specific exception --- src/System.Management.Automation/engine/Utils.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/System.Management.Automation/engine/Utils.cs b/src/System.Management.Automation/engine/Utils.cs index 5cac8729279..2a0996ce92e 100644 --- a/src/System.Management.Automation/engine/Utils.cs +++ b/src/System.Management.Automation/engine/Utils.cs @@ -1406,7 +1406,7 @@ internal static Encoding GetEncoding(string path) return sr.CurrentEncoding; } } - catch (System.Exception) + catch (IOException) { return ClrFacade.GetDefaultEncoding(); } From 4390247bb55bd83e4d3be123817f88814f6d3cc3 Mon Sep 17 00:00:00 2001 From: Xavier Hahn Date: Wed, 28 Oct 2020 12:37:57 +0100 Subject: [PATCH 04/10] Further cleanup and fix PR comment Remove the GetEncoding from Utils and two related static readonly fields. Add the default to utf8NoBom in the stream reader Put the stream reader in the try catch. --- .../engine/Utils.cs | 36 ------------------- .../engine/hostifaces/MshHostUserInterface.cs | 12 +++++-- 2 files changed, 9 insertions(+), 39 deletions(-) diff --git a/src/System.Management.Automation/engine/Utils.cs b/src/System.Management.Automation/engine/Utils.cs index 2a0996ce92e..44ef54bc9c3 100644 --- a/src/System.Management.Automation/engine/Utils.cs +++ b/src/System.Management.Automation/engine/Utils.cs @@ -1391,47 +1391,11 @@ internal static bool Succeeded(int hresult) return hresult >= 0; } - // Attempt to determine the existing encoding - internal static Encoding GetEncoding(string path) - { - if (!File.Exists(path)) - { - return ClrFacade.GetDefaultEncoding(); - } - - try - { - using (StreamReader sr = new StreamReader(path, true)) - { - return sr.CurrentEncoding; - } - } - catch (IOException) - { - return ClrFacade.GetDefaultEncoding(); - } - } - // BigEndianUTF32 encoding is possible, but requires creation internal static readonly Encoding BigEndianUTF32Encoding = new UTF32Encoding(bigEndian: true, byteOrderMark: true); // [System.Text.Encoding]::GetEncodings() | Where-Object { $_.GetEncoding().GetPreamble() } | // Add-Member ScriptProperty Preamble { $this.GetEncoding().GetPreamble() -join "-" } -PassThru | // Format-Table -Auto - internal static readonly Dictionary encodingMap = - new Dictionary() - { - { "255-254", Encoding.Unicode }, - { "254-255", Encoding.BigEndianUnicode }, - { "255-254-0-0", Encoding.UTF32 }, - { "0-0-254-255", BigEndianUTF32Encoding }, - { "239-187-191", Encoding.UTF8 }, - }; - - internal static readonly char[] nonPrintableCharacters = { - (char) 0, (char) 1, (char) 2, (char) 3, (char) 4, (char) 5, (char) 6, (char) 7, (char) 8, - (char) 11, (char) 12, (char) 14, (char) 15, (char) 16, (char) 17, (char) 18, (char) 19, (char) 20, - (char) 21, (char) 22, (char) 23, (char) 24, (char) 25, (char) 26, (char) 28, (char) 29, (char) 30, - (char) 31, (char) 127, (char) 129, (char) 141, (char) 143, (char) 144, (char) 157 }; internal static readonly UTF8Encoding utf8NoBom = new UTF8Encoding(encoderShouldEmitUTF8Identifier: false); diff --git a/src/System.Management.Automation/engine/hostifaces/MshHostUserInterface.cs b/src/System.Management.Automation/engine/hostifaces/MshHostUserInterface.cs index 05d5819b6b6..429107c5252 100644 --- a/src/System.Management.Automation/engine/hostifaces/MshHostUserInterface.cs +++ b/src/System.Management.Automation/engine/hostifaces/MshHostUserInterface.cs @@ -1099,14 +1099,20 @@ internal void FlushContentToDisk() { if (_contentWriter == null) { - var encoding = Utils.GetEncoding(this.Path); try { + Encoding currentEncoding = null; + + using (StreamReader reader = new StreamReader(this.Path, Utils.utf8NoBom, true)) + { + currentEncoding = reader.CurrentEncoding; + } + // Try to first open the file with permissions that will allow us to read from it // later. _contentWriter = new StreamWriter( new FileStream(this.Path, FileMode.OpenOrCreate, FileAccess.ReadWrite, FileShare.Read), - encoding); + currentEncoding ?? Utils.utf8NoBom); _contentWriter.BaseStream.Seek(0, SeekOrigin.End); } catch (IOException) @@ -1115,7 +1121,7 @@ internal void FlushContentToDisk() // file permissions. _contentWriter = new StreamWriter( new FileStream(this.Path, FileMode.Append, FileAccess.Write, FileShare.Read), - encoding); + Utils.utf8NoBom); } _contentWriter.AutoFlush = true; From 3185744c9850f5551001265b97727c9cef92ec60 Mon Sep 17 00:00:00 2001 From: Xavier Hahn Date: Wed, 28 Oct 2020 14:32:06 +0100 Subject: [PATCH 05/10] Implement PR suggestions --- .../engine/hostifaces/MshHostUserInterface.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/System.Management.Automation/engine/hostifaces/MshHostUserInterface.cs b/src/System.Management.Automation/engine/hostifaces/MshHostUserInterface.cs index 429107c5252..2149b23177e 100644 --- a/src/System.Management.Automation/engine/hostifaces/MshHostUserInterface.cs +++ b/src/System.Management.Automation/engine/hostifaces/MshHostUserInterface.cs @@ -1101,10 +1101,11 @@ internal void FlushContentToDisk() { try { - Encoding currentEncoding = null; + Encoding currentEncoding = Utils.utf8NoBom; - using (StreamReader reader = new StreamReader(this.Path, Utils.utf8NoBom, true)) + using (StreamReader reader = new StreamReader(this.Path, Utils.utf8NoBom, detectEncodingFromByteOrderMarks: true)) { + _ = reader.Read(); currentEncoding = reader.CurrentEncoding; } @@ -1112,7 +1113,7 @@ internal void FlushContentToDisk() // later. _contentWriter = new StreamWriter( new FileStream(this.Path, FileMode.OpenOrCreate, FileAccess.ReadWrite, FileShare.Read), - currentEncoding ?? Utils.utf8NoBom); + currentEncoding); _contentWriter.BaseStream.Seek(0, SeekOrigin.End); } catch (IOException) From 6c42ba2d975eab4ddf401c2d1dca97dde7b33062 Mon Sep 17 00:00:00 2001 From: Xavier Hahn Date: Wed, 28 Oct 2020 16:31:37 +0100 Subject: [PATCH 06/10] There is no need to initialize the variable --- .../engine/hostifaces/MshHostUserInterface.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/System.Management.Automation/engine/hostifaces/MshHostUserInterface.cs b/src/System.Management.Automation/engine/hostifaces/MshHostUserInterface.cs index 2149b23177e..6b100a8e04b 100644 --- a/src/System.Management.Automation/engine/hostifaces/MshHostUserInterface.cs +++ b/src/System.Management.Automation/engine/hostifaces/MshHostUserInterface.cs @@ -1101,7 +1101,7 @@ internal void FlushContentToDisk() { try { - Encoding currentEncoding = Utils.utf8NoBom; + Encoding currentEncoding; using (StreamReader reader = new StreamReader(this.Path, Utils.utf8NoBom, detectEncodingFromByteOrderMarks: true)) { From 64c1407849211ae1f231bd9bae82e159e6a2c4bb Mon Sep 17 00:00:00 2001 From: Xavier Hahn Date: Tue, 10 Nov 2020 22:13:32 +0100 Subject: [PATCH 07/10] Factor get path encoding Use a local function to factor the path encoding code. --- .../engine/hostifaces/MshHostUserInterface.cs | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/src/System.Management.Automation/engine/hostifaces/MshHostUserInterface.cs b/src/System.Management.Automation/engine/hostifaces/MshHostUserInterface.cs index 6b100a8e04b..6e6ab11c8fe 100644 --- a/src/System.Management.Automation/engine/hostifaces/MshHostUserInterface.cs +++ b/src/System.Management.Automation/engine/hostifaces/MshHostUserInterface.cs @@ -1101,13 +1101,7 @@ internal void FlushContentToDisk() { try { - Encoding currentEncoding; - - using (StreamReader reader = new StreamReader(this.Path, Utils.utf8NoBom, detectEncodingFromByteOrderMarks: true)) - { - _ = reader.Read(); - currentEncoding = reader.CurrentEncoding; - } + var currentEncoding = getPathEncoding(); // Try to first open the file with permissions that will allow us to read from it // later. @@ -1136,6 +1130,15 @@ internal void FlushContentToDisk() OutputBeingLogged.Clear(); } + + Encoding getPathEncoding() + { + using (StreamReader reader = new StreamReader(this.Path, Utils.utf8NoBom, detectEncodingFromByteOrderMarks: true)) + { + _ = reader.Read(); + return reader.CurrentEncoding; + } + } } private StreamWriter _contentWriter = null; From 6c6bbf4df1b4aa43b909a09e58725b2fa82662a8 Mon Sep 17 00:00:00 2001 From: Xavier Hahn Date: Wed, 11 Nov 2020 21:20:47 +0100 Subject: [PATCH 08/10] Moved local function to start of method and PascalCase --- .../engine/hostifaces/MshHostUserInterface.cs | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/System.Management.Automation/engine/hostifaces/MshHostUserInterface.cs b/src/System.Management.Automation/engine/hostifaces/MshHostUserInterface.cs index 6e6ab11c8fe..d83e7a1bcca 100644 --- a/src/System.Management.Automation/engine/hostifaces/MshHostUserInterface.cs +++ b/src/System.Management.Automation/engine/hostifaces/MshHostUserInterface.cs @@ -1093,6 +1093,15 @@ internal TranscriptionOption() /// internal void FlushContentToDisk() { + Encoding GetPathEncoding() + { + using (StreamReader reader = new StreamReader(this.Path, Utils.utf8NoBom, detectEncodingFromByteOrderMarks: true)) + { + _ = reader.Read(); + return reader.CurrentEncoding; + } + } + lock (OutputBeingLogged) { if (!_disposed) @@ -1101,7 +1110,7 @@ internal void FlushContentToDisk() { try { - var currentEncoding = getPathEncoding(); + var currentEncoding = GetPathEncoding(); // Try to first open the file with permissions that will allow us to read from it // later. @@ -1130,15 +1139,6 @@ internal void FlushContentToDisk() OutputBeingLogged.Clear(); } - - Encoding getPathEncoding() - { - using (StreamReader reader = new StreamReader(this.Path, Utils.utf8NoBom, detectEncodingFromByteOrderMarks: true)) - { - _ = reader.Read(); - return reader.CurrentEncoding; - } - } } private StreamWriter _contentWriter = null; From 6c4833860584787c0b77e0810e085225deebb24a Mon Sep 17 00:00:00 2001 From: Xavier Hahn Date: Fri, 13 Nov 2020 11:39:05 +0100 Subject: [PATCH 09/10] Make GetPathEncoding static and use inline using Co-authored-by: Ilya --- .../engine/hostifaces/MshHostUserInterface.cs | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/System.Management.Automation/engine/hostifaces/MshHostUserInterface.cs b/src/System.Management.Automation/engine/hostifaces/MshHostUserInterface.cs index d83e7a1bcca..dc933d5c481 100644 --- a/src/System.Management.Automation/engine/hostifaces/MshHostUserInterface.cs +++ b/src/System.Management.Automation/engine/hostifaces/MshHostUserInterface.cs @@ -1093,13 +1093,11 @@ internal TranscriptionOption() /// internal void FlushContentToDisk() { - Encoding GetPathEncoding() + static Encoding GetPathEncoding(string path) { - using (StreamReader reader = new StreamReader(this.Path, Utils.utf8NoBom, detectEncodingFromByteOrderMarks: true)) - { - _ = reader.Read(); - return reader.CurrentEncoding; - } + using StreamReader reader = new StreamReader(path, Utils.utf8NoBom, detectEncodingFromByteOrderMarks: true); + _ = reader.Read(); + return reader.CurrentEncoding; } lock (OutputBeingLogged) @@ -1322,4 +1320,3 @@ internal static int DetermineChoicePicked(string response, Collection Date: Fri, 13 Nov 2020 16:47:10 +0100 Subject: [PATCH 10/10] Add back missing parameter to GetPathEncoding Path Co-authored-by: Ilya --- .../engine/hostifaces/MshHostUserInterface.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/System.Management.Automation/engine/hostifaces/MshHostUserInterface.cs b/src/System.Management.Automation/engine/hostifaces/MshHostUserInterface.cs index dc933d5c481..75f55d625f8 100644 --- a/src/System.Management.Automation/engine/hostifaces/MshHostUserInterface.cs +++ b/src/System.Management.Automation/engine/hostifaces/MshHostUserInterface.cs @@ -1108,7 +1108,7 @@ static Encoding GetPathEncoding(string path) { try { - var currentEncoding = GetPathEncoding(); + var currentEncoding = GetPathEncoding(this.Path); // Try to first open the file with permissions that will allow us to read from it // later.