Skip to content

Commit

Permalink
Fix Issue #257 for insane program names
Browse files Browse the repository at this point in the history 
This change replaces the Name and ShortName on ProgramDescription in XML serialization with the MetadataString class.  This fixes issue #257, which is a crash due to bad data being parsed from ROM metadata.

The contributing factors were:
1. Bug parsing metadata from scrambled ROMs (fixed previously)
2. Bad behaviors in XmlSerializer - valid UNICODE strings containing non-XML-safe characters can be serialized, but not de-serialized
3. intvname would pass along arbitrary bytes

These are addressed in different ways.
Item 1 was addressed in commit 5446f34.

Item 3 was fixed in jzintv as of SVN revision 1840, though subsequent changes have improved upon the UTF-8 support. Those changes in jzintv result in 'unusual' characters being escaped when written to to .cfg files or returned via the intvname utility.

Item 2 is addressed in this change, via the MetadataString class and the altering of how ProgramDescription is serialized via XML.

The Copy test for ProgramDescription was updated to include the new XmlName and XmlShortName properties, as well as setting a non-trivial ShortName that requires escaping the characters.

Also, the XmlSerializer parsing tests were updated to test strings that needed to be escaped.  These are somewhat redundant, in that MetadataString already covers serialization of itself.  The new tests here validate the behaviors not of the XML parsing, so much as the correct API surface of the ProgramDescription class itself -- namely that its Name and ShortName properties behave correctly.

Further validation was done with hand testing on a large, existing ROM and menu layout.
  • Loading branch information
@intvsteve
intvsteve committed Feb 22, 2019
1 parent e20fa4b commit 55eac50
Show file tree
Hide file tree
Showing 2 changed files with 196 additions and 3 deletions.
61 changes: 59 additions & 2 deletions INTV.Core/Model/Program/ProgramDescription.cs
View file
Expand Up @@ -30,6 +30,13 @@ namespace INTV.Core.Model.Program
/// </summary>
public class ProgramDescription : INTV.Core.ComponentModel.ModelBase, IProgramDescription
{
#region Property Names

public const string NamePropertyName = "Name";
public const string ShortNamePropertyName = "ShortName";

#endregion // Property Names

/// <summary>
/// Maximum length for a program name.
/// </summary>
Expand Down Expand Up @@ -59,7 +66,9 @@ public ProgramDescription(uint crc, IRom rom, IProgramInformation programInfo)
_rom = rom;
_programInfo = new UserSpecifiedProgramInformation(programInfo);
_name = _programInfo.GetNameForCrc(crc);
_xmlName = new MetadataString() { Text = _name };
_shortName = programInfo.ShortName;
_xmlShortName = new MetadataString() { Text = _shortName };
_programFiles = new ProgramSupportFiles(rom);
var allIncompatibilities = _programInfo.Crcs.Select(c => c.Incompatibilities);
var crcData = _programInfo.Crcs.First(c => c.Crc == crc);
Expand Down Expand Up @@ -119,6 +128,8 @@ public IProgramInformation ProgramInformation
private IProgramInformation _programInfo;

/// <inheritdoc />
/// <remarks>For purposes of XML serialization, the XmlName property is used.</remarks>
[System.Xml.Serialization.XmlIgnore]
public string Name
{
get
Expand All @@ -130,13 +141,15 @@ public string Name
{
if (!string.IsNullOrWhiteSpace(value))
{
this.AssignAndUpdateProperty("Name", value.EnforceNameLength(MaxProgramNameLength, false), ref _name, (p, v) => UpdateNameFromXml(v));
this.AssignAndUpdateProperty(NamePropertyName, value.EnforceNameLength(MaxProgramNameLength, false), ref _name, (p, v) => UpdateNameFromXml(v));
}
}
}
private string _name;

/// <inheritdoc />
/// <remarks>For purposes of XML serialization, the XmlShortName property is used.</remarks>
[System.Xml.Serialization.XmlIgnore]
public string ShortName
{
get
Expand All @@ -146,7 +159,7 @@ public string ShortName

set
{
this.AssignAndUpdateProperty("ShortName", value.EnforceNameLength(RomInfoIndexHelpers.MaxShortNameLength, false), ref _shortName, (p, v) => UpdateShortNameFromXml(v));
this.AssignAndUpdateProperty(ShortNamePropertyName, value.EnforceNameLength(RomInfoIndexHelpers.MaxShortNameLength, false), ref _shortName, (p, v) => UpdateShortNameFromXml(v));
}
}
private string _shortName;
Expand Down Expand Up @@ -213,6 +226,46 @@ public string Code
}
#endif // false

/// <summary>
/// XML-safe version of the <see cref="Name"/> property.
/// </summary>
/// <remarks>This should only be accessed via the XmlSerializer.</remarks>
[System.Xml.Serialization.XmlElement(NamePropertyName)]
public MetadataString XmlName
{
get
{
return _xmlName;
}

set
{
_xmlName = value;
Name = _xmlName.Text;
}
}
private MetadataString _xmlName;

/// <summary>
/// XML-safe version of the <see cref="Name"/> property.
/// </summary>
/// <remarks>This should only be accessed via the XmlSerializer.</remarks>
[System.Xml.Serialization.XmlElement(ShortNamePropertyName)]
public MetadataString XmlShortName
{
get
{
return _xmlShortName;
}

set
{
_xmlShortName = value;
ShortName = _xmlShortName.Text;
}
}
private MetadataString _xmlShortName;

#endregion // Properties

/// <summary>
Expand Down Expand Up @@ -277,7 +330,9 @@ public ProgramDescription Copy()
{
var description = new ProgramDescription(Crc, Rom, ProgramInformation);
description._name = _name;
description._xmlName = new MetadataString() { Text = _name };
description._shortName = _shortName;
description._xmlShortName = new MetadataString() { Text = _shortName };
description._programFiles = _programFiles.Copy();
return description;
}
Expand Down Expand Up @@ -345,6 +400,7 @@ private void UpdateRomFromXml(ProgramSupportFiles supportFiles)

private void UpdateNameFromXml(string name)
{
XmlName.Text = name;
var info = _programInfo as UserSpecifiedProgramInformation;
if (info != null)
{
Expand All @@ -354,6 +410,7 @@ private void UpdateNameFromXml(string name)

private void UpdateShortNameFromXml(string shortName)
{
XmlShortName.Text = shortName;
var info = _programInfo as UserSpecifiedProgramInformation;
if (info != null)
{
Expand Down
138 changes: 137 additions & 1 deletion Tests/INTV.Core.Tests/Model/Program/ProgramDescriptionTests.cs
View file
@@ -1,4 +1,4 @@
// <copyright file="ProgramDescriptionTests.cs" company="INTV Funhouse">
// <copyright file="ProgramDescriptionTests.cs" company="INTV Funhouse">
// Copyright (c) 2018-2019 All Rights Reserved
// <author>Steven A. Orth</author>
//
Expand Down Expand Up @@ -563,13 +563,19 @@ public void ProgramDescription_Copy_ProducesEquivalentCopy()
information.AddCrc(crc, name);

var description0 = new ProgramDescription(crc, null, information);
description0.ShortName = "A\aa";
var description1 = description0.Copy();

Assert.Equal(description0.Name, description1.Name);
Assert.Equal(description0.ShortName, description1.ShortName);
Assert.Equal(description0.Vendor, description1.Vendor);
Assert.Equal(description0.Year, description1.Year);
Assert.Equal(description0.Features, description1.Features);
Assert.Equal(description0.XmlName.XmlText, description1.XmlName.XmlText);
Assert.Equal(description0.XmlName.Escaped, description1.XmlName.Escaped);
Assert.Equal(description0.XmlShortName.XmlText, description1.XmlShortName.XmlText);
Assert.Equal(description0.XmlShortName.Escaped, description1.XmlShortName.Escaped);
Assert.Equal(description0.Vendor, description1.Vendor);
Assert.True(object.ReferenceEquals(description0.Rom, description1.Rom));
VerifyProgramInformation(description0.ProgramInformation, description1.ProgramInformation);
VerifyProgramSupportFiles(description0.Files, description1.Files);
Expand Down Expand Up @@ -640,6 +646,136 @@ public void ProgramDescription_ParseFromXml_ProducesValidProgramDescription()
Assert.Equal(@"/Users/tester/Projects/MinGW/msys/1.0/home/tester/lui_src/LtoFlash/bin/tools/0.cfg", description.Rom.ConfigPath);
}

[Fact]
public void ProgramDescription_ParseFromXmlWithBinHexName_ProducesValidProgramDescription()
{
ProgramDescriptionTestStorage.Initialize(null);
var xmlProgramDescription = @"<?xml version=""1.0""?>
<ProgramDescription>
<Crc>3971627767</Crc>
<Name Escaped=""true"">546167616C6F6E67200720546F646421</Name>
<ShortName>T.T.</ShortName>
<Vendor>Zbiciak Electronics</Vendor>
<Year>1999</Year>
<Features>
<Ntsc>Tolerates</Ntsc>
<Pal>Tolerates</Pal>
<GeneralFeatures>None</GeneralFeatures>
<KeyboardComponent>Tolerates</KeyboardComponent>
<SuperVideoArcade>Tolerates</SuperVideoArcade>
<Intellivoice>Tolerates</Intellivoice>
<IntellivisionII>Tolerates</IntellivisionII>
<Ecs>Tolerates</Ecs>
<Tutorvision>Tolerates</Tutorvision>
<Intellicart>Tolerates</Intellicart>
<CuttleCart3>Tolerates</CuttleCart3>
<Jlp>Incompatible</Jlp>
<JlpHardwareVersion>None</JlpHardwareVersion>
<JlpFlashMinimumSaveSectors>0</JlpFlashMinimumSaveSectors>
<LtoFlash>Incompatible</LtoFlash>
<Bee3>Incompatible</Bee3>
<Hive>Incompatible</Hive>
</Features>
<Files>
<RomImagePath>\Users\tester\Projects\perforce\intellivision\roms\tagalong.bin</RomImagePath>
<RomConfigurationFilePath>/Users/tester/Projects/MinGW/msys/1.0/home/tester/lui_src/LtoFlash/bin/tools/0.cfg</RomConfigurationFilePath>
<AlternateRomImagePaths />
<AlternateRomConfigurationFilePaths />
</Files>
</ProgramDescription>
";
ProgramDescription description;
using (var stream = new MemoryStream(Encoding.UTF8.GetBytes(xmlProgramDescription)))
{
var serializer = new XmlSerializer(typeof(ProgramDescription));
description = serializer.Deserialize(stream) as ProgramDescription;
}

Assert.NotNull(description);
Assert.Equal("Tagalong \a Todd!", description.Name);
Assert.Equal("T.T.", description.ShortName);
Assert.Equal("Zbiciak Electronics", description.Vendor);
Assert.Equal("1999", description.Year);
Assert.Equal(TestRomResources.TestBinCrc, description.Crc);
Assert.Equal(ProgramFeatures.DefaultFeatures, description.Features);
Assert.Equal(@"\Users\tester\Projects\perforce\intellivision\roms\tagalong.bin", description.Files.RomImagePath);
Assert.Equal(@"/Users/tester/Projects/MinGW/msys/1.0/home/tester/lui_src/LtoFlash/bin/tools/0.cfg", description.Files.RomConfigurationFilePath);
Assert.Empty(description.Files.AlternateRomImagePaths);
Assert.Empty(description.Files.AlternateRomConfigurationFilePaths);
Assert.NotNull(description.Rom);
Assert.True(description.Rom is XmlRom);
Assert.Null(((XmlRom)description.Rom).ResolvedRom);
Assert.Equal(0u, description.Rom.Crc);
Assert.Equal(0u, description.Rom.CfgCrc);
Assert.Equal(@"\Users\tester\Projects\perforce\intellivision\roms\tagalong.bin", description.Rom.RomPath);
Assert.Equal(@"/Users/tester/Projects/MinGW/msys/1.0/home/tester/lui_src/LtoFlash/bin/tools/0.cfg", description.Rom.ConfigPath);
}

[Fact]
public void ProgramDescription_ParseFromXmlWithBinHexShortName_ProducesValidProgramDescription()
{
ProgramDescriptionTestStorage.Initialize(null);
var xmlProgramDescription = @"<?xml version=""1.0""?>
<ProgramDescription>
<Crc>3971627767</Crc>
<Name>546167616C6F6E67200720546F646421</Name>
<ShortName Escaped=""true"">546167616C6F6E67200720546F646421</ShortName>
<Vendor>Zbiciak Electronics</Vendor>
<Year>1999</Year>
<Features>
<Ntsc>Tolerates</Ntsc>
<Pal>Tolerates</Pal>
<GeneralFeatures>None</GeneralFeatures>
<KeyboardComponent>Tolerates</KeyboardComponent>
<SuperVideoArcade>Tolerates</SuperVideoArcade>
<Intellivoice>Tolerates</Intellivoice>
<IntellivisionII>Tolerates</IntellivisionII>
<Ecs>Tolerates</Ecs>
<Tutorvision>Tolerates</Tutorvision>
<Intellicart>Tolerates</Intellicart>
<CuttleCart3>Tolerates</CuttleCart3>
<Jlp>Incompatible</Jlp>
<JlpHardwareVersion>None</JlpHardwareVersion>
<JlpFlashMinimumSaveSectors>0</JlpFlashMinimumSaveSectors>
<LtoFlash>Incompatible</LtoFlash>
<Bee3>Incompatible</Bee3>
<Hive>Incompatible</Hive>
</Features>
<Files>
<RomImagePath>\Users\tester\Projects\perforce\intellivision\roms\tagalong.bin</RomImagePath>
<RomConfigurationFilePath>/Users/tester/Projects/MinGW/msys/1.0/home/tester/lui_src/LtoFlash/bin/tools/0.cfg</RomConfigurationFilePath>
<AlternateRomImagePaths />
<AlternateRomConfigurationFilePaths />
</Files>
</ProgramDescription>
";
ProgramDescription description;
using (var stream = new MemoryStream(Encoding.UTF8.GetBytes(xmlProgramDescription)))
{
var serializer = new XmlSerializer(typeof(ProgramDescription));
description = serializer.Deserialize(stream) as ProgramDescription;
}

Assert.NotNull(description);
Assert.Equal("546167616C6F6E67200720546F646421", description.Name);
Assert.Equal("Tagalong \a Todd!", description.ShortName);
Assert.Equal("Zbiciak Electronics", description.Vendor);
Assert.Equal("1999", description.Year);
Assert.Equal(TestRomResources.TestBinCrc, description.Crc);
Assert.Equal(ProgramFeatures.DefaultFeatures, description.Features);
Assert.Equal(@"\Users\tester\Projects\perforce\intellivision\roms\tagalong.bin", description.Files.RomImagePath);
Assert.Equal(@"/Users/tester/Projects/MinGW/msys/1.0/home/tester/lui_src/LtoFlash/bin/tools/0.cfg", description.Files.RomConfigurationFilePath);
Assert.Empty(description.Files.AlternateRomImagePaths);
Assert.Empty(description.Files.AlternateRomConfigurationFilePaths);
Assert.NotNull(description.Rom);
Assert.True(description.Rom is XmlRom);
Assert.Null(((XmlRom)description.Rom).ResolvedRom);
Assert.Equal(0u, description.Rom.Crc);
Assert.Equal(0u, description.Rom.CfgCrc);
Assert.Equal(@"\Users\tester\Projects\perforce\intellivision\roms\tagalong.bin", description.Rom.RomPath);
Assert.Equal(@"/Users/tester/Projects/MinGW/msys/1.0/home/tester/lui_src/LtoFlash/bin/tools/0.cfg", description.Rom.ConfigPath);
}

private void VerifyProgramInformation(IProgramInformation information0, IProgramInformation information1)
{
Assert.Equal(information0.DataOrigin, information1.DataOrigin);
Expand Down

0 comments on commit 55eac50

Please sign in to comment.