From f04992fab584fd0738ca20ec115b1b6b9dd185d7 Mon Sep 17 00:00:00 2001 From: Pranav K Date: Sat, 13 Jun 2020 20:39:41 -0700 Subject: [PATCH] Add nullable to DataProtection (#22591) * Add nullable to DataProtection Contributes to https://github.com/dotnet/aspnetcore/issues/5680 --- ...NetCore.DataProtection.Abstractions.csproj | 1 + .../src/DataProtectionCommonExtensions.cs | 4 +- src/DataProtection/Abstractions/src/Error.cs | 2 +- ...NetCore.DataProtection.Abstractions.csproj | 3 +- ...ft.AspNetCore.Cryptography.Internal.csproj | 1 + .../Cryptography.Internal/src/CryptoUtil.cs | 3 +- ...ft.AspNetCore.Cryptography.Internal.csproj | 3 +- ...pNetCore.Cryptography.KeyDerivation.csproj | 1 + ...pNetCore.Cryptography.KeyDerivation.csproj | 4 +- ...Microsoft.AspNetCore.DataProtection.csproj | 1 + ...Microsoft.AspNetCore.DataProtection.csproj | 1 + .../src/DataProtectionKey.cs | 6 +-- .../src/EntityFrameworkCoreXmlRepository.cs | 37 +++++++++---------- .../src/LoggingExtensions.cs | 18 ++++----- ....DataProtection.EntityFrameworkCore.csproj | 3 +- ...spNetCore.DataProtection.Extensions.csproj | 1 + .../Extensions/src/DataProtectionProvider.cs | 4 +- ...spNetCore.DataProtection.Extensions.csproj | 3 +- .../src/TimeLimitedDataProtector.cs | 2 +- ...e.DataProtection.StackExchangeRedis.csproj | 3 +- src/Shared/WebEncoders/WebEncoders.cs | 1 - 21 files changed, 57 insertions(+), 45 deletions(-) diff --git a/src/DataProtection/Abstractions/ref/Microsoft.AspNetCore.DataProtection.Abstractions.csproj b/src/DataProtection/Abstractions/ref/Microsoft.AspNetCore.DataProtection.Abstractions.csproj index a317d80c11..2f58874ab1 100644 --- a/src/DataProtection/Abstractions/ref/Microsoft.AspNetCore.DataProtection.Abstractions.csproj +++ b/src/DataProtection/Abstractions/ref/Microsoft.AspNetCore.DataProtection.Abstractions.csproj @@ -3,6 +3,7 @@ netstandard2.0;$(DefaultNetCoreTargetFramework) $(DefaultNetCoreTargetFramework) + annotations diff --git a/src/DataProtection/Abstractions/src/DataProtectionCommonExtensions.cs b/src/DataProtection/Abstractions/src/DataProtectionCommonExtensions.cs index f4fd8801ae..3b7951fcf3 100644 --- a/src/DataProtection/Abstractions/src/DataProtectionCommonExtensions.cs +++ b/src/DataProtection/Abstractions/src/DataProtectionCommonExtensions.cs @@ -88,7 +88,7 @@ namespace Microsoft.AspNetCore.DataProtection // because we don't want the code provider.CreateProtector() [parameterless] to inadvertently compile. // The actual signature for this method forces at least one purpose to be provided at the call site. - IDataProtector protector = provider.CreateProtector(purpose); + IDataProtector? protector = provider.CreateProtector(purpose); if (subPurposes != null && subPurposes.Length > 0) { protector = protector?.CreateProtector((IEnumerable)subPurposes); @@ -111,7 +111,7 @@ namespace Microsoft.AspNetCore.DataProtection // We have our own implementation of GetRequiredService since we don't want to // take a dependency on DependencyInjection.Interfaces. - IDataProtectionProvider provider = (IDataProtectionProvider)services.GetService(typeof(IDataProtectionProvider)); + var provider = (IDataProtectionProvider?)services.GetService(typeof(IDataProtectionProvider)); if (provider == null) { throw new InvalidOperationException(Resources.FormatDataProtectionExtensions_NoService(typeof(IDataProtectionProvider).FullName)); diff --git a/src/DataProtection/Abstractions/src/Error.cs b/src/DataProtection/Abstractions/src/Error.cs index 18b93c0ac7..eba12d15c9 100644 --- a/src/DataProtection/Abstractions/src/Error.cs +++ b/src/DataProtection/Abstractions/src/Error.cs @@ -9,7 +9,7 @@ namespace Microsoft.AspNetCore.DataProtection { internal static class Error { - public static CryptographicException CryptCommon_GenericError(Exception inner = null) + public static CryptographicException CryptCommon_GenericError(Exception? inner = null) { return new CryptographicException(Resources.CryptCommon_GenericError, inner); } diff --git a/src/DataProtection/Abstractions/src/Microsoft.AspNetCore.DataProtection.Abstractions.csproj b/src/DataProtection/Abstractions/src/Microsoft.AspNetCore.DataProtection.Abstractions.csproj index dcbaf568ee..160fff97e3 100644 --- a/src/DataProtection/Abstractions/src/Microsoft.AspNetCore.DataProtection.Abstractions.csproj +++ b/src/DataProtection/Abstractions/src/Microsoft.AspNetCore.DataProtection.Abstractions.csproj @@ -1,4 +1,4 @@ - + ASP.NET Core data protection abstractions. @@ -9,6 +9,7 @@ Microsoft.AspNetCore.DataProtection.IDataProtector true true aspnetcore;dataprotection + enable diff --git a/src/DataProtection/Cryptography.Internal/ref/Microsoft.AspNetCore.Cryptography.Internal.csproj b/src/DataProtection/Cryptography.Internal/ref/Microsoft.AspNetCore.Cryptography.Internal.csproj index 5ea2ac05b7..7bba415573 100644 --- a/src/DataProtection/Cryptography.Internal/ref/Microsoft.AspNetCore.Cryptography.Internal.csproj +++ b/src/DataProtection/Cryptography.Internal/ref/Microsoft.AspNetCore.Cryptography.Internal.csproj @@ -3,6 +3,7 @@ netstandard2.0;$(DefaultNetCoreTargetFramework) $(DefaultNetCoreTargetFramework) + annotations diff --git a/src/DataProtection/Cryptography.Internal/src/CryptoUtil.cs b/src/DataProtection/Cryptography.Internal/src/CryptoUtil.cs index e60673634d..853b50e3f8 100644 --- a/src/DataProtection/Cryptography.Internal/src/CryptoUtil.cs +++ b/src/DataProtection/Cryptography.Internal/src/CryptoUtil.cs @@ -3,6 +3,7 @@ using System; using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; using System.Runtime.CompilerServices; using System.Runtime.ConstrainedExecution; using System.Runtime.InteropServices; @@ -16,7 +17,7 @@ namespace Microsoft.AspNetCore.Cryptography { // This isn't a typical Debug.Assert; the check is always performed, even in retail builds. [MethodImpl(MethodImplOptions.AggressiveInlining)] - public static void Assert(bool condition, string message) + public static void Assert([DoesNotReturnIf(false)] bool condition, string message) { if (!condition) { diff --git a/src/DataProtection/Cryptography.Internal/src/Microsoft.AspNetCore.Cryptography.Internal.csproj b/src/DataProtection/Cryptography.Internal/src/Microsoft.AspNetCore.Cryptography.Internal.csproj index 88ab0de27e..b23c38e9e5 100644 --- a/src/DataProtection/Cryptography.Internal/src/Microsoft.AspNetCore.Cryptography.Internal.csproj +++ b/src/DataProtection/Cryptography.Internal/src/Microsoft.AspNetCore.Cryptography.Internal.csproj @@ -1,4 +1,4 @@ - + Infrastructure for ASP.NET Core cryptographic packages. Applications and libraries should not reference this package directly. @@ -8,6 +8,7 @@ true true aspnetcore;dataprotection + annotations diff --git a/src/DataProtection/Cryptography.KeyDerivation/ref/Microsoft.AspNetCore.Cryptography.KeyDerivation.csproj b/src/DataProtection/Cryptography.KeyDerivation/ref/Microsoft.AspNetCore.Cryptography.KeyDerivation.csproj index 93587ee4e0..d8f6699fa2 100644 --- a/src/DataProtection/Cryptography.KeyDerivation/ref/Microsoft.AspNetCore.Cryptography.KeyDerivation.csproj +++ b/src/DataProtection/Cryptography.KeyDerivation/ref/Microsoft.AspNetCore.Cryptography.KeyDerivation.csproj @@ -3,6 +3,7 @@ netstandard2.0;$(DefaultNetCoreTargetFramework) $(DefaultNetCoreTargetFramework) + annotations diff --git a/src/DataProtection/Cryptography.KeyDerivation/src/Microsoft.AspNetCore.Cryptography.KeyDerivation.csproj b/src/DataProtection/Cryptography.KeyDerivation/src/Microsoft.AspNetCore.Cryptography.KeyDerivation.csproj index 4ca096eb00..48e0bf5f5f 100644 --- a/src/DataProtection/Cryptography.KeyDerivation/src/Microsoft.AspNetCore.Cryptography.KeyDerivation.csproj +++ b/src/DataProtection/Cryptography.KeyDerivation/src/Microsoft.AspNetCore.Cryptography.KeyDerivation.csproj @@ -1,4 +1,4 @@ - + ASP.NET Core utilities for key derivation. @@ -7,6 +7,8 @@ true true aspnetcore;dataprotection + enable + annotations diff --git a/src/DataProtection/DataProtection/ref/Microsoft.AspNetCore.DataProtection.csproj b/src/DataProtection/DataProtection/ref/Microsoft.AspNetCore.DataProtection.csproj index c3d2e237c7..4d03b6cea7 100644 --- a/src/DataProtection/DataProtection/ref/Microsoft.AspNetCore.DataProtection.csproj +++ b/src/DataProtection/DataProtection/ref/Microsoft.AspNetCore.DataProtection.csproj @@ -3,6 +3,7 @@ netstandard2.0;$(DefaultNetCoreTargetFramework) $(DefaultNetCoreTargetFramework) + annotations diff --git a/src/DataProtection/DataProtection/src/Microsoft.AspNetCore.DataProtection.csproj b/src/DataProtection/DataProtection/src/Microsoft.AspNetCore.DataProtection.csproj index a675b5aa74..de50517328 100644 --- a/src/DataProtection/DataProtection/src/Microsoft.AspNetCore.DataProtection.csproj +++ b/src/DataProtection/DataProtection/src/Microsoft.AspNetCore.DataProtection.csproj @@ -9,6 +9,7 @@ true true aspnetcore;dataprotection + annotations diff --git a/src/DataProtection/EntityFrameworkCore/src/DataProtectionKey.cs b/src/DataProtection/EntityFrameworkCore/src/DataProtectionKey.cs index c236d5cb89..b59ee4f6c6 100644 --- a/src/DataProtection/EntityFrameworkCore/src/DataProtectionKey.cs +++ b/src/DataProtection/EntityFrameworkCore/src/DataProtectionKey.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. namespace Microsoft.AspNetCore.DataProtection.EntityFrameworkCore @@ -16,11 +16,11 @@ namespace Microsoft.AspNetCore.DataProtection.EntityFrameworkCore /// /// The friendly name of the . /// - public string FriendlyName { get; set; } + public string? FriendlyName { get; set; } /// /// The XML representation of the . /// - public string Xml { get; set; } + public string? Xml { get; set; } } } diff --git a/src/DataProtection/EntityFrameworkCore/src/EntityFrameworkCoreXmlRepository.cs b/src/DataProtection/EntityFrameworkCore/src/EntityFrameworkCoreXmlRepository.cs index 3af024996a..ec9efba809 100644 --- a/src/DataProtection/EntityFrameworkCore/src/EntityFrameworkCoreXmlRepository.cs +++ b/src/DataProtection/EntityFrameworkCore/src/EntityFrameworkCoreXmlRepository.cs @@ -40,13 +40,25 @@ namespace Microsoft.AspNetCore.DataProtection.EntityFrameworkCore /// public virtual IReadOnlyCollection GetAllElements() { - using (var scope = _services.CreateScope()) - { - var context = scope.ServiceProvider.GetRequiredService(); + // forces complete enumeration + return GetAllElementsCore().ToList().AsReadOnly(); - // Put logger in a local such that `this` isn't captured. - var logger = _logger; - return context.DataProtectionKeys.AsNoTracking().Select(key => TryParseKeyXml(key.Xml, logger)).ToList().AsReadOnly(); + IEnumerable GetAllElementsCore() + { + using (var scope = _services.CreateScope()) + { + var context = scope.ServiceProvider.GetRequiredService(); + + foreach (var key in context.DataProtectionKeys.AsNoTracking()) + { + _logger.ReadingXmlFromKey(key.FriendlyName!, key.Xml); + + if (!string.IsNullOrEmpty(key.Xml)) + { + yield return XElement.Parse(key.Xml); + } + } + } } } @@ -67,18 +79,5 @@ namespace Microsoft.AspNetCore.DataProtection.EntityFrameworkCore context.SaveChanges(); } } - - private static XElement TryParseKeyXml(string xml, ILogger logger) - { - try - { - return XElement.Parse(xml); - } - catch (Exception e) - { - logger?.LogExceptionWhileParsingKeyXml(xml, e); - return null; - } - } } } diff --git a/src/DataProtection/EntityFrameworkCore/src/LoggingExtensions.cs b/src/DataProtection/EntityFrameworkCore/src/LoggingExtensions.cs index 193c0100ac..894d0a3051 100644 --- a/src/DataProtection/EntityFrameworkCore/src/LoggingExtensions.cs +++ b/src/DataProtection/EntityFrameworkCore/src/LoggingExtensions.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; @@ -7,23 +7,23 @@ namespace Microsoft.Extensions.Logging { internal static class LoggingExtensions { - private static readonly Action _anExceptionOccurredWhileParsingKeyXml; - private static readonly Action _savingKeyToDbContext; + private static readonly Action _readingXmlFromKey; + private static readonly Action _savingKeyToDbContext; static LoggingExtensions() { - _anExceptionOccurredWhileParsingKeyXml = LoggerMessage.Define( - eventId: new EventId(1, "ExceptionOccurredWhileParsingKeyXml"), - logLevel: LogLevel.Warning, - formatString: "An exception occurred while parsing the key xml '{Xml}'."); + _readingXmlFromKey = LoggerMessage.Define( + eventId: new EventId(1, "ReadKeyFromElement"), + logLevel: LogLevel.Debug, + formatString: "Reading data with key '{FriendlyName}', value '{Value}'."); _savingKeyToDbContext = LoggerMessage.Define( eventId: new EventId(2, "SavingKeyToDbContext"), logLevel: LogLevel.Debug, formatString: "Saving key '{FriendlyName}' to '{DbContext}'."); } - public static void LogExceptionWhileParsingKeyXml(this ILogger logger, string keyXml, Exception exception) - => _anExceptionOccurredWhileParsingKeyXml(logger, keyXml, exception); + public static void ReadingXmlFromKey(this ILogger logger, string? friendlyName, string? keyXml) + => _readingXmlFromKey(logger, friendlyName, keyXml, null); public static void LogSavingKeyToDbContext(this ILogger logger, string friendlyName, string contextName) => _savingKeyToDbContext(logger, friendlyName, contextName, null); diff --git a/src/DataProtection/EntityFrameworkCore/src/Microsoft.AspNetCore.DataProtection.EntityFrameworkCore.csproj b/src/DataProtection/EntityFrameworkCore/src/Microsoft.AspNetCore.DataProtection.EntityFrameworkCore.csproj index f76d1568c3..63c9ad80dc 100644 --- a/src/DataProtection/EntityFrameworkCore/src/Microsoft.AspNetCore.DataProtection.EntityFrameworkCore.csproj +++ b/src/DataProtection/EntityFrameworkCore/src/Microsoft.AspNetCore.DataProtection.EntityFrameworkCore.csproj @@ -1,4 +1,4 @@ - + Support for storing keys using Entity Framework Core. @@ -7,6 +7,7 @@ true aspnetcore;dataprotection;entityframeworkcore true + enable diff --git a/src/DataProtection/Extensions/ref/Microsoft.AspNetCore.DataProtection.Extensions.csproj b/src/DataProtection/Extensions/ref/Microsoft.AspNetCore.DataProtection.Extensions.csproj index df813aaf5f..305ca2cc9f 100644 --- a/src/DataProtection/Extensions/ref/Microsoft.AspNetCore.DataProtection.Extensions.csproj +++ b/src/DataProtection/Extensions/ref/Microsoft.AspNetCore.DataProtection.Extensions.csproj @@ -3,6 +3,7 @@ netstandard2.0;$(DefaultNetCoreTargetFramework) $(DefaultNetCoreTargetFramework) + annotations diff --git a/src/DataProtection/Extensions/src/DataProtectionProvider.cs b/src/DataProtection/Extensions/src/DataProtectionProvider.cs index cc82fe9ef8..0ffd6d0d96 100644 --- a/src/DataProtection/Extensions/src/DataProtectionProvider.cs +++ b/src/DataProtection/Extensions/src/DataProtectionProvider.cs @@ -151,9 +151,9 @@ namespace Microsoft.AspNetCore.DataProtection } internal static IDataProtectionProvider CreateProvider( - DirectoryInfo keyDirectory, + DirectoryInfo? keyDirectory, Action setupAction, - X509Certificate2 certificate) + X509Certificate2? certificate) { // build the service collection var serviceCollection = new ServiceCollection(); diff --git a/src/DataProtection/Extensions/src/Microsoft.AspNetCore.DataProtection.Extensions.csproj b/src/DataProtection/Extensions/src/Microsoft.AspNetCore.DataProtection.Extensions.csproj index fe1e5448da..a8947d0584 100644 --- a/src/DataProtection/Extensions/src/Microsoft.AspNetCore.DataProtection.Extensions.csproj +++ b/src/DataProtection/Extensions/src/Microsoft.AspNetCore.DataProtection.Extensions.csproj @@ -1,4 +1,4 @@ - + Additional APIs for ASP.NET Core data protection. @@ -7,6 +7,7 @@ true true aspnetcore;dataprotection + enable diff --git a/src/DataProtection/Extensions/src/TimeLimitedDataProtector.cs b/src/DataProtection/Extensions/src/TimeLimitedDataProtector.cs index 71e9c3c553..174176d143 100644 --- a/src/DataProtection/Extensions/src/TimeLimitedDataProtector.cs +++ b/src/DataProtection/Extensions/src/TimeLimitedDataProtector.cs @@ -17,7 +17,7 @@ namespace Microsoft.AspNetCore.DataProtection private const string MyPurposeString = "Microsoft.AspNetCore.DataProtection.TimeLimitedDataProtector.v1"; private readonly IDataProtector _innerProtector; - private IDataProtector _innerProtectorWithTimeLimitedPurpose; // created on-demand + private IDataProtector? _innerProtectorWithTimeLimitedPurpose; // created on-demand public TimeLimitedDataProtector(IDataProtector innerProtector) { diff --git a/src/DataProtection/StackExchangeRedis/src/Microsoft.AspNetCore.DataProtection.StackExchangeRedis.csproj b/src/DataProtection/StackExchangeRedis/src/Microsoft.AspNetCore.DataProtection.StackExchangeRedis.csproj index b32253856f..ab7553df96 100644 --- a/src/DataProtection/StackExchangeRedis/src/Microsoft.AspNetCore.DataProtection.StackExchangeRedis.csproj +++ b/src/DataProtection/StackExchangeRedis/src/Microsoft.AspNetCore.DataProtection.StackExchangeRedis.csproj @@ -1,4 +1,4 @@ - + Support for storing data protection keys in Redis. @@ -7,6 +7,7 @@ true aspnetcore;dataprotection;redis true + enable diff --git a/src/Shared/WebEncoders/WebEncoders.cs b/src/Shared/WebEncoders/WebEncoders.cs index c3e403889d..7ce03ec1a4 100644 --- a/src/Shared/WebEncoders/WebEncoders.cs +++ b/src/Shared/WebEncoders/WebEncoders.cs @@ -2,7 +2,6 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. #nullable enable - using System; #if NETCOREAPP using System.Buffers;