Skip to content

Commit

Permalink
(chocolateyGH-1505) Ensure package information is escaped
Browse files Browse the repository at this point in the history
- Also, when de-serializing, if a .backup file exists, remove original
- Warn user that .backup file exists and needs to be corrected
  • Loading branch information
gep13 committed Feb 23, 2018
1 parent 9393c95 commit cd33f4a
Show file tree
Hide file tree
Showing 5 changed files with 123 additions and 74 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,11 @@ public static string get_value_as_string(this RegistryKey key, string name)
{
if (key == null) return string.Empty;

return key.GetValue(name).to_string().Replace("\0", string.Empty);
// Since it is possible that registry keys contain characters that are not valid
// in XML files, ensure that all content is escaped, prior to serialization
var escapedXml = System.Security.SecurityElement.Escape(key.GetValue(name).to_string());

return escapedXml == null ? string.Empty : escapedXml.Replace("\0", string.Empty);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

namespace chocolatey.infrastructure.app.services
{
using System;
using System.IO;
using System.Text;
using NuGet;
Expand All @@ -29,6 +30,7 @@ public class ChocolateyPackageInformationService : IChocolateyPackageInformation
private readonly IRegistryService _registryService;
private readonly IFilesService _filesService;
private const string REGISTRY_SNAPSHOT_FILE = ".registry";
private const string REGISTRY_SNAPSHOT_BAD_FILE = ".registry.bad";
private const string FILES_SNAPSHOT_FILE = ".files";
private const string SILENT_UNINSTALLER_FILE = ".silentUninstaller";
private const string SIDE_BY_SIDE_FILE = ".sxs";
Expand Down Expand Up @@ -59,16 +61,44 @@ public ChocolateyPackageInformation get_package_information(IPackage package)
return packageInformation;
}

FaultTolerance.try_catch_with_logging_exception(
() =>
var deserializationErrorMessage = @"
A corrupt .registry file exists at {0}.
Open this file in a text editor, and remove/escape any characters that
are regarded as illegal within XML contents. These are typically the
characters <, >, "", ', and &. Once these have been corrected, rename
the .registry.bad file to .registry. Once saved, try running the same
Chocolatey command that was just executed, so verify problem is fixed.
NOTE: It will not be possible to rename the file in Windows Explorer.
Instead, you can use the following PowerShell command:
Move-Item .\.registry.bad .\.registry
".format_with(_fileSystem.combine_paths(pkgStorePath, REGISTRY_SNAPSHOT_BAD_FILE));
try
{
if (_fileSystem.file_exists(_fileSystem.combine_paths(pkgStorePath, REGISTRY_SNAPSHOT_BAD_FILE)))
{
this.Log().Warn(deserializationErrorMessage);
}
else
{
packageInformation.RegistrySnapshot = _registryService.read_from_file(_fileSystem.combine_paths(pkgStorePath, REGISTRY_SNAPSHOT_FILE));
}
}
catch (Exception)
{
FaultTolerance.try_catch_with_logging_exception(
() =>
{
packageInformation.RegistrySnapshot = _registryService.read_from_file(_fileSystem.combine_paths(pkgStorePath, REGISTRY_SNAPSHOT_FILE));
this.Log().Warn(deserializationErrorMessage);

// rename the bad registry file so that it isn't processed again
_fileSystem.move_file(_fileSystem.combine_paths(pkgStorePath, REGISTRY_SNAPSHOT_FILE), _fileSystem.combine_paths(pkgStorePath, REGISTRY_SNAPSHOT_BAD_FILE));
},
"Unable to read registry snapshot file for {0} (located at {1})".format_with(package.Id, _fileSystem.combine_paths(pkgStorePath, REGISTRY_SNAPSHOT_FILE)),
throwError: false,
logWarningInsteadOfError: true,
isSilent: true
);
);
}

FaultTolerance.try_catch_with_logging_exception(
() =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -398,7 +398,7 @@ public Registry read_from_file(string filePath)
return null;
}

return _xmlService.deserialize<Registry>(filePath);
return _xmlService.deserialize<Registry>(filePath, 1);
}

private void get_values(RegistryKey key, string subKeyName, IList<GenericRegistryValue> values, bool expandValues)
Expand Down
9 changes: 9 additions & 0 deletions src/chocolatey/infrastructure/services/IXmlService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,15 @@ public interface IXmlService
/// <returns></returns>
XmlType deserialize<XmlType>(string xmlFilePath);

/// <summary>
/// Deserializes the specified XML file path.
/// </summary>
/// <typeparam name="XmlType">The type of the ml type.</typeparam>
/// <param name="xmlFilePath">The XML file path.</param>
/// <param name="retryCount">The number of times to attempt deserialization on event of a failure.</param>
/// <returns></returns>
XmlType deserialize<XmlType>(string xmlFilePath, int retryCount);

/// <summary>
/// Serializes the specified XML type.
/// </summary>
Expand Down
142 changes: 74 additions & 68 deletions src/chocolatey/infrastructure/services/XmlService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -45,75 +45,81 @@ public XmlService(IFileSystem fileSystem, IHashProvider hashProvider)

public XmlType deserialize<XmlType>(string xmlFilePath)
{
return FaultTolerance.retry(3, () => GlobalMutex.enter(
() =>
{
this.Log().Trace("Entered mutex to deserialize '{0}'".format_with(xmlFilePath));

return FaultTolerance.try_catch_with_logging_exception(
() =>
{
var xmlSerializer = new XmlSerializer(typeof(XmlType));
using (var fileStream = _fileSystem.open_file_readonly(xmlFilePath))
using (var fileReader = new StreamReader(fileStream))
using (var xmlReader = XmlReader.Create(fileReader))
{
if (!xmlSerializer.CanDeserialize(xmlReader))
{
this.Log().Warn("Cannot deserialize response of type {0}", typeof(XmlType));
return default(XmlType);
}

try
{
return (XmlType)xmlSerializer.Deserialize(xmlReader);
}
catch (InvalidOperationException ex)
{
// Check if its just a malformed document.
if (ex.Message.Contains("There is an error in XML document"))
{
// If so, check for a backup file and try an parse that.
if (_fileSystem.file_exists(xmlFilePath + ".backup"))
{
using (var backupStream = _fileSystem.open_file_readonly(xmlFilePath + ".backup"))
using (var backupReader = new StreamReader(backupStream))
using (var backupXmlReader = XmlReader.Create(backupReader))
{
var validConfig = (XmlType)xmlSerializer.Deserialize(backupXmlReader);

// If there's no errors and it's valid, go ahead and replace the bad file with the backup.
if (validConfig != null)
{
_fileSystem.copy_file(xmlFilePath + ".backup", xmlFilePath, overwriteExisting: true);
}

return validConfig;
}
}
}

throw;
}
finally
{
foreach (var updateFile in _fileSystem.get_files(_fileSystem.get_directory_name(xmlFilePath), "*.update").or_empty_list_if_null())
{
this.Log().Debug("Removing '{0}'".format_with(updateFile));
FaultTolerance.try_catch_with_logging_exception(
() => _fileSystem.delete_file(updateFile),
errorMessage: "Unable to remove update file",
logDebugInsteadOfError: true,
isSilent: true
);
}
}
}
},
"Error deserializing response of type {0}".format_with(typeof(XmlType)),
throwError: true);
return deserialize<XmlType>(xmlFilePath, 3);
}

}, MUTEX_TIMEOUT),
public XmlType deserialize<XmlType>(string xmlFilePath, int retryCount)
{
return FaultTolerance.retry(retryCount, () => GlobalMutex.enter(
() =>
{
this.Log().Trace("Entered mutex to deserialize '{0}'".format_with(xmlFilePath));

return FaultTolerance.try_catch_with_logging_exception(
() =>
{
var xmlSerializer = new XmlSerializer(typeof(XmlType));
using (var fileStream = _fileSystem.open_file_readonly(xmlFilePath))
using (var fileReader = new StreamReader(fileStream))
using (var xmlReader = XmlReader.Create(fileReader))
{
if (!xmlSerializer.CanDeserialize(xmlReader))
{
this.Log().Warn("Cannot deserialize response of type {0}", typeof(XmlType));
return default(XmlType);
}

try
{
return (XmlType)xmlSerializer.Deserialize(xmlReader);
}
catch (InvalidOperationException ex)
{
// Check if its just a malformed document.
if (ex.Message.Contains("There is an error in XML document"))
{
// If so, check for a backup file and try an parse that.
if (_fileSystem.file_exists(xmlFilePath + ".backup"))
{
using (var backupStream = _fileSystem.open_file_readonly(xmlFilePath + ".backup"))
using (var backupReader = new StreamReader(backupStream))
using (var backupXmlReader = XmlReader.Create(backupReader))
{
var validConfig = (XmlType)xmlSerializer.Deserialize(backupXmlReader);

// If there's no errors and it's valid, go ahead and replace the bad file with the backup.
if (validConfig != null)
{
_fileSystem.copy_file(xmlFilePath + ".backup", xmlFilePath, overwriteExisting: true);
}

return validConfig;
}
}
}

throw;

}
finally
{
foreach (var updateFile in _fileSystem.get_files(_fileSystem.get_directory_name(xmlFilePath), "*.update").or_empty_list_if_null())
{
this.Log().Debug("Removing '{0}'".format_with(updateFile));
FaultTolerance.try_catch_with_logging_exception(
() => _fileSystem.delete_file(updateFile),
errorMessage: "Unable to remove update file",
logDebugInsteadOfError: true,
isSilent: true
);
}
}
}
},
"Error deserializing response of type {0}".format_with(typeof(XmlType)),
throwError: true);

}, MUTEX_TIMEOUT),
waitDurationMilliseconds: 200,
increaseRetryByMilliseconds: 200);
}
Expand Down

0 comments on commit cd33f4a

Please sign in to comment.