From 73c9a0989ffe1d2e2ff42c7195b3cf3cd12a78aa Mon Sep 17 00:00:00 2001 From: Jared Parsons Date: Fri, 4 Jan 2019 11:22:21 -0800 Subject: [PATCH] Respond to PR feedback --- .../Core/Portable/Compilation.EmitStream.cs | 33 ++++++++++++++----- .../Core/Portable/Compilation/Compilation.cs | 12 +++---- .../StrongName/DesktopStrongNameProvider.cs | 2 -- 3 files changed, 29 insertions(+), 18 deletions(-) diff --git a/src/Compilers/Core/Portable/Compilation.EmitStream.cs b/src/Compilers/Core/Portable/Compilation.EmitStream.cs index 8a1d88fb001..6f3c14d9104 100644 --- a/src/Compilers/Core/Portable/Compilation.EmitStream.cs +++ b/src/Compilers/Core/Portable/Compilation.EmitStream.cs @@ -3,6 +3,8 @@ using System; using System.Diagnostics; using System.IO; +using System.Reflection.PortableExecutable; +using Microsoft.CodeAnalysis.Interop; using Roslyn.Utilities; namespace Microsoft.CodeAnalysis @@ -16,7 +18,20 @@ public abstract partial class Compilation internal enum EmitStreamSignKind { None, + + /// + /// This form of signing occurs in memory using the APIs. This is the default + /// form of signing and will be used when a strong name key is provided in a file on disk. + /// SignedWithBulider, + + /// + /// This form of signing occurs using the COM APIs. This form of signing + /// requires the unsigned PE to be written to disk before it can be signed (typically by writing it + /// out to the %TEMP% folder). This signing is used when the key in a key container, the signing + /// requires a counter signature or customers opted in via the UseLegacyStrongNameProvider feature + /// flag. + /// SignedWithFile, } @@ -34,7 +49,7 @@ internal sealed class EmitStream /// /// The that is being emitted into. This value should _never_ be - /// Dispose. It is either returned from the instance in + /// disposed. It is either returned from the instance in /// which case it is owned by that. Or it is just an alias for the value that is stored /// in in which case it will be disposed from there. /// @@ -83,7 +98,6 @@ private Stream CreateStream(DiagnosticBag diagnostics) _stream = _emitStreamProvider.GetOrCreateStream(diagnostics); if (_stream == null) { - Debug.Assert(diagnostics.HasAnyErrors()); return null; } @@ -95,22 +109,24 @@ private Stream CreateStream(DiagnosticBag diagnostics) { Debug.Assert(_strongNameProvider != null); + Stream tempStream; + string tempFilePath; try { var fileSystem = _strongNameProvider.FileSystem; Func streamConstructor = path => fileSystem.CreateFileStream(path, FileMode.Create, FileAccess.ReadWrite, FileShare.ReadWrite); var tempDir = fileSystem.GetTempPath(); - var tempFilePath = Path.Combine(tempDir, Guid.NewGuid().ToString("N")); - var tempStream = FileUtilities.CreateFileStreamChecked(streamConstructor, tempFilePath); - - _tempInfo = (tempStream, tempFilePath); - return tempStream; + tempFilePath = Path.Combine(tempDir, Guid.NewGuid().ToString("N")); + tempStream = FileUtilities.CreateFileStreamChecked(streamConstructor, tempFilePath); } catch (IOException e) { throw new Cci.PeWritingException(e); } + + _tempInfo = (tempStream, tempFilePath); + return tempStream; } else { @@ -121,11 +137,12 @@ private Stream CreateStream(DiagnosticBag diagnostics) internal bool Complete(StrongNameKeys strongNameKeys, CommonMessageProvider messageProvider, DiagnosticBag diagnostics) { Debug.Assert(_stream != null); + Debug.Assert(_emitStreamSignKind != EmitStreamSignKind.SignedWithFile || _tempInfo.HasValue); if (_tempInfo.HasValue) { Debug.Assert(_emitStreamSignKind == EmitStreamSignKind.SignedWithFile); - var (tempStream, tempFilePath) = _tempInfo.Value; + var (tempStream, tempFilePath) = _tempInfo.GetValueOrDefault(); try { diff --git a/src/Compilers/Core/Portable/Compilation/Compilation.cs b/src/Compilers/Core/Portable/Compilation/Compilation.cs index 71317e6cf75..e2a15729f32 100644 --- a/src/Compilers/Core/Portable/Compilation/Compilation.cs +++ b/src/Compilers/Core/Portable/Compilation/Compilation.cs @@ -1658,7 +1658,7 @@ internal void SetupWin32Resources(CommonPEModuleBuilder moduleBeingBuilt, Stream /// 2. Write the unsigned PE to disk and use CLR COM APIs to sign. /// The preferred method is #1 as it's more efficient and more resilient (no reliance on %TEMP%). But /// we must continue to support #2 as it's the only way to do the following: - /// - Access private keys stored in a key + /// - Access private keys stored in a key container /// - Do proper counter signature verification for AssemblySignatureKey attributes /// internal bool SignUsingBuilder => @@ -2732,11 +2732,9 @@ internal void EnsureAnonymousTypeTemplates(CancellationToken cancellationToken) return false; } - if (!emitPeStream.Complete(StrongNameKeys, MessageProvider, diagnostics) || - emitMetadataStream?.Complete(StrongNameKeys, MessageProvider, diagnostics) == false) - { - return false; - } + return + emitPeStream.Complete(StrongNameKeys, MessageProvider, diagnostics) && + (emitMetadataStream?.Complete(StrongNameKeys, MessageProvider, diagnostics) ?? true); } finally { @@ -2746,8 +2744,6 @@ internal void EnsureAnonymousTypeTemplates(CancellationToken cancellationToken) pdbBag?.Free(); metadataDiagnostics?.Free(); } - - return true; } private static Stream ConditionalGetOrCreateStream(EmitStreamProvider metadataPEStreamProvider, DiagnosticBag metadataDiagnostics) diff --git a/src/Compilers/Core/Portable/StrongName/DesktopStrongNameProvider.cs b/src/Compilers/Core/Portable/StrongName/DesktopStrongNameProvider.cs index b420ad7ac38..8e2f2b9ba51 100644 --- a/src/Compilers/Core/Portable/StrongName/DesktopStrongNameProvider.cs +++ b/src/Compilers/Core/Portable/StrongName/DesktopStrongNameProvider.cs @@ -29,7 +29,6 @@ public class DesktopStrongNameProvider : StrongNameProvider // so there's no chance of an API consumer seeing it. internal sealed class ClrStrongNameMissingException : Exception { - } private readonly ImmutableArray _keyFileSearchPaths; @@ -37,7 +36,6 @@ internal sealed class ClrStrongNameMissingException : Exception public DesktopStrongNameProvider(ImmutableArray keyFileSearchPaths) : this(keyFileSearchPaths, StrongNameFileSystem.Instance) { - } /// -- GitLab