From 1092b6c105326d44cd8cf9e139a23170c6fb2b8e Mon Sep 17 00:00:00 2001 From: TheCakeIsNaOH Date: Tue, 11 Jan 2022 22:44:01 -0600 Subject: [PATCH] (#747) Switch CryptoHashProvider to filestream for hashing files Previously, when getting the checksum of a file, this read the file into a byte array to pass to ComputeHash. However, this was limited to files of 2gb or less, and was not efficent memory wise. This changes the method to using a filestream which is passed to ComputeHash, which should allow arbitrary file sizes with better memory usage. The unit test for this method had to be removed because the mock filesystem does not allow opening a file stream. --- src/chocolatey.tests/chocolatey.tests.csproj | 1 - .../cryptography/CryptoHashProviderSpecs.cs | 110 ------------------ .../ApplicationParameters.cs | 1 - .../cryptography/CryptoHashProvider.cs | 14 ++- 4 files changed, 8 insertions(+), 118 deletions(-) delete mode 100644 src/chocolatey.tests/infrastructure/cryptography/CryptoHashProviderSpecs.cs diff --git a/src/chocolatey.tests/chocolatey.tests.csproj b/src/chocolatey.tests/chocolatey.tests.csproj index e37e7d0216..785bf6561c 100644 --- a/src/chocolatey.tests/chocolatey.tests.csproj +++ b/src/chocolatey.tests/chocolatey.tests.csproj @@ -201,7 +201,6 @@ - diff --git a/src/chocolatey.tests/infrastructure/cryptography/CryptoHashProviderSpecs.cs b/src/chocolatey.tests/infrastructure/cryptography/CryptoHashProviderSpecs.cs deleted file mode 100644 index db4685c48e..0000000000 --- a/src/chocolatey.tests/infrastructure/cryptography/CryptoHashProviderSpecs.cs +++ /dev/null @@ -1,110 +0,0 @@ -// Copyright © 2017 - 2021 Chocolatey Software, Inc -// Copyright © 2011 - 2017 RealDimensions Software, LLC -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -namespace chocolatey.tests.infrastructure.cryptography -{ - using System; - using System.IO; - using System.Security.Cryptography; - using chocolatey.infrastructure.adapters; - using chocolatey.infrastructure.app; - using chocolatey.infrastructure.cryptography; - using chocolatey.infrastructure.filesystem; - using Moq; - using FluentAssertions; - - public class CryptoHashProviderSpecs - { - public abstract class CryptoHashProviderSpecsBase : TinySpec - { - protected CryptoHashProvider Provider; - protected Mock FileSystem = new Mock(); - - public override void Context() - { - Provider = Provider = new CryptoHashProvider(FileSystem.Object); - } - } - - public class When_HashProvider_provides_a_hash : CryptoHashProviderSpecsBase - { - private string result; - private readonly string filePath = "c:\\path\\does\\not\\matter.txt"; - private readonly byte[] byteArray = new byte[] { 23, 25, 27 }; - - public override void Context() - { - base.Context(); - FileSystem.Setup(x => x.FileExists(It.IsAny())).Returns(true); - FileSystem.Setup(x => x.ReadFileBytes(filePath)).Returns(byteArray); - } - - public override void Because() - { - result = Provider.ComputeFileHash(filePath); - } - - [Fact] - public void Should_provide_the_correct_hash_based_on_a_checksum() - { - var expected = BitConverter.ToString(SHA256.Create().ComputeHash(byteArray)).Replace("-", string.Empty); - - result.Should().Be(expected); - } - } - - public class When_HashProvider_attempts_to_provide_a_hash_for_a_file_over_2GB : CryptoHashProviderSpecsBase - { - private string result; - private readonly string filePath = "c:\\path\\does\\not\\matter.txt"; - private readonly byte[] byteArray = new byte[] { 23, 25, 27 }; - private readonly Mock _hashAlgorithm = new Mock(); - - public override void Context() - { - base.Context(); - Provider = new CryptoHashProvider(FileSystem.Object, _hashAlgorithm.Object); - - FileSystem.Setup(x => x.FileExists(It.IsAny())).Returns(true); - FileSystem.Setup(x => x.ReadFileBytes(filePath)).Returns(byteArray); - _hashAlgorithm.Setup(x => x.ComputeHash(byteArray)).Throws(); //IO.IO_FileTooLong2GB (over Int32.MaxValue) - } - - public override void Because() - { - result = Provider.ComputeFileHash(filePath); - } - - [Fact] - public void Should_log_a_warning() - { - MockLogger.MessagesFor(LogLevel.Warn).Should().HaveCount(1); - } - - [Fact] - public void Should_not_throw_an_error_itself() - { - //this handles itself - } - - [Fact] - public void Should_provide_an_unchanging_hash_for_a_file_too_big_to_hash() - { - result.Should().Be(ApplicationParameters.HashProviderFileTooBig); - } - } - } -} diff --git a/src/chocolatey/infrastructure.app/ApplicationParameters.cs b/src/chocolatey/infrastructure.app/ApplicationParameters.cs index 122e6ddfa9..753f35020b 100644 --- a/src/chocolatey/infrastructure.app/ApplicationParameters.cs +++ b/src/chocolatey/infrastructure.app/ApplicationParameters.cs @@ -156,7 +156,6 @@ public static class Environment public static readonly string ConfigFileTransformExtension = ".install.xdt"; public static readonly string[] ShimDirectorFileExtensions = new string[] {".gui",".ignore"}; - public static readonly string HashProviderFileTooBig = "UnableToDetectChanges_FileTooBig"; public static readonly string HashProviderFileLocked = "UnableToDetectChanges_FileLocked"; /// diff --git a/src/chocolatey/infrastructure/cryptography/CryptoHashProvider.cs b/src/chocolatey/infrastructure/cryptography/CryptoHashProvider.cs index a88e0bc929..a67d30cd2c 100644 --- a/src/chocolatey/infrastructure/cryptography/CryptoHashProvider.cs +++ b/src/chocolatey/infrastructure/cryptography/CryptoHashProvider.cs @@ -90,21 +90,23 @@ public string ComputeFileHash(string filePath) try { - var hash = _hashAlgorithm.ComputeHash(_fileSystem.ReadFileBytes(filePath)); - - return BitConverter.ToString(hash).Replace("-", string.Empty); + using (var fileStream = _fileSystem.open_file_readonly(filePath)) + { + var hash = _hashAlgorithm.ComputeHash(fileStream); + return BitConverter.ToString(hash).Replace("-", string.Empty); + } } catch (IOException ex) { - this.Log().Warn(() => "Error computing hash for '{0}'{1} Hash will be special code for locked file or file too big instead.{1} Captured error:{1} {2}".FormatWith(filePath, Environment.NewLine, ex.Message)); + this.Log().Warn(() => "Error computing hash for '{0}'{1} Hash will be special code for locked file {1} Captured error:{1} {2}".FormatWith(filePath, Environment.NewLine, ex.Message)); if (IsFileLocked(ex)) { return ApplicationParameters.HashProviderFileLocked; } - //IO.IO_FileTooLong2GB (over Int32.MaxValue) - return ApplicationParameters.HashProviderFileTooBig; + //Rethrow if file is not locked + throw; } }