Do not modify stream position in ZipPluginPackageReader (#327)

* Do not modify stream position in ZipPluginPackageReader

* Check stream canread and canseek

* Remove catching ArgumentException when creating zip archive
This commit is contained in:
helenkzhang 2023-09-22 14:15:08 -07:00 коммит произвёл GitHub
Родитель 336bca857f
Коммит 71d6ae3eff
Не найден ключ, соответствующий данной подписи
Идентификатор ключа GPG: 4AEE18F83AFDEB23
2 изменённых файлов: 27 добавлений и 17 удалений

Просмотреть файл

@ -49,7 +49,7 @@ namespace Microsoft.Performance.Toolkit.Plugins.Runtime.Tests
var package = await sut.TryReadPackageAsync(stream, CancellationToken.None); var package = await sut.TryReadPackageAsync(stream, CancellationToken.None);
Assert.IsNull(package); Assert.IsNull(package);
fakeLogger.Verify(logger => logger.Error(It.IsAny<ArgumentException>(), It.IsAny<string>())); fakeLogger.Verify(logger => logger.Error(It.IsAny<string>()));
} }
[TestMethod] [TestMethod]

Просмотреть файл

@ -57,27 +57,35 @@ namespace Microsoft.Performance.Toolkit.Plugins.Runtime.Package
{ {
Guard.NotNull(stream, nameof(stream)); Guard.NotNull(stream, nameof(stream));
if (!stream.CanRead)
{
this.logger.Error($"The stream is not readable.");
return null;
}
if (!stream.CanSeek)
{
this.logger.Error($"The stream is not seekable.");
return null;
}
bool success = false; bool success = false;
long originalPosition = stream.Position;
// Try to open the stream as a zip archive // Try to open the stream as a zip archive
ZipArchive zip; ZipArchive zip = null;
try try
{ {
zip = new ZipArchive(stream, ZipArchiveMode.Read, true); try
} {
catch (ArgumentException e) zip = new ZipArchive(stream, ZipArchiveMode.Read, true);
{ }
this.logger.Error(e, $"Unable to read from the stream."); catch (InvalidDataException e)
return null; {
} this.logger.Error(e, $"The stream could not be interpreted as a zip archive.");
catch (InvalidDataException e) return null;
{ }
this.logger.Error(e, $"The stream could not be interpreted as a zip archive.");
return null;
}
try
{
// Check that the plugin content folder exists // Check that the plugin content folder exists
bool hasContentFolder = zip.Entries.Any( bool hasContentFolder = zip.Entries.Any(
e => e.FullName.Replace('\\', '/').StartsWith(PackageConstants.PluginContentFolderName, StringComparison.OrdinalIgnoreCase)); e => e.FullName.Replace('\\', '/').StartsWith(PackageConstants.PluginContentFolderName, StringComparison.OrdinalIgnoreCase));
@ -139,7 +147,9 @@ namespace Microsoft.Performance.Toolkit.Plugins.Runtime.Package
} }
finally finally
{ {
if (!success) stream.Position = originalPosition;
if (!success && zip != null)
{ {
zip.Dispose(); zip.Dispose();
} }