Merge pull request #102 from Numpsy/rw/null_termination

Ensure string values are null terminated when writing them

sources/OpenMcdf.Extensions/OLEProperties/PropertyFactory.cs

@@ -417,11 +417,21 @@ public override string ReadScalarValue(System.IO.BinaryReader br)
 
             public override void WriteScalarValue(BinaryWriter bw, string pValue)
             {
+                data = Encoding.GetEncoding(codePage).GetBytes(pValue);
+                uint dataLength = (uint)data.Length;
 
-                data = Encoding.GetEncoding(codePage).GetBytes((String)pValue);
+                // The string data must be null terminated, so add a null byte if there isn't one already
+                bool addNullTerminator =
+                    dataLength == 0 || data[dataLength - 1] != '\0';
 
-                bw.Write((uint)data.Length);
+                if (addNullTerminator) 
+                    dataLength += 1;
+
+                bw.Write(dataLength);
                 bw.Write(data);
+
+                if (addNullTerminator)
+                    bw.Write((byte)0);
             }
         }
 
@@ -446,8 +456,24 @@ public override string ReadScalarValue(System.IO.BinaryReader br)
             public override void WriteScalarValue(BinaryWriter bw, string pValue)
             {
                 data = Encoding.Unicode.GetBytes(pValue);
-                bw.Write((uint)data.Length >> 2);
+
+                // The written data length field is the number of characters (not bytes) and must include a null terminator
+                // add a null terminator if there isn't one already
+                var byteLength = data.Length;
+                bool addNullTerminator =
+                    byteLength == 0 || data[byteLength - 1] != '\0' || data[byteLength - 2] != '\0';
+
+                if (addNullTerminator)
+                    byteLength += 2;
+
+                bw.Write((uint)byteLength / 2);
                 bw.Write(data);
+
+                if (addNullTerminator)
+                {
+                    bw.Write((byte)0);
+                    bw.Write((byte)0);
+                }
             }
         }
 

sources/Test/OpenMcdf.Extensions.Test/OLEPropertiesExtensionsTest.cs

@@ -3,6 +3,7 @@
 using System.IO;
 using Microsoft.VisualStudio.TestTools.UnitTesting;
 using System.Linq;
+using OpenMcdf.Extensions.OLEProperties;
 
 namespace OpenMcdf.Extensions.Test
 {
@@ -127,6 +128,54 @@ public void Test_DOCUMENT_SUMMARY_INFO_ROUND_TRIP()
 
         }
 
+        // Modify some document summary information properties, save to a file, and then validate the expected results
+        [TestMethod]
+        public void Test_DOCUMENT_SUMMARY_INFO_MODIFY()
+        {
+            if (File.Exists("test_modify_summary.ppt"))
+                File.Delete("test_modify_summary.ppt");
+
+            // Verify initial properties, and then create a modified document
+            using (CompoundFile cf = new CompoundFile("_Test.ppt"))
+            {
+                var dsiStream = cf.RootStorage.GetStream("\u0005DocumentSummaryInformation");
+                var co = dsiStream.AsOLEPropertiesContainer();
+
+                // The company property should exist but be empty
+                var companyProperty = co.Properties.First(prop => prop.PropertyName == "PIDDSI_COMPANY");
+                Assert.AreEqual("\0\0\0\0", companyProperty.Value);
+
+                // As a sanity check, check that the value of a property that we don't change remains the same
+                var formatProperty = co.Properties.First(prop => prop.PropertyName == "PIDDSI_PRESFORMAT");
+                Assert.AreEqual("A4 Paper (210x297 mm)\0\0\0", formatProperty.Value);
+
+                // The manager property shouldn't exist, and we'll add it
+                Assert.IsFalse(co.Properties.Any(prop => prop.PropertyName == "PIDDSI_MANAGER"));
+                var managerProp = co.NewProperty(VTPropertyType.VT_LPSTR, 0x0000000E, "PIDDSI_MANAGER");
+                co.AddProperty(managerProp);
+
+                companyProperty.Value = "My Company";
+                managerProp.Value = "The Boss";
+
+                co.Save(dsiStream);
+                cf.SaveAs(@"test_modify_summary.ppt");
+            }
+
+            using (CompoundFile cf = new CompoundFile("test_modify_summary.ppt"))
+            {
+                var co = cf.RootStorage.GetStream("\u0005DocumentSummaryInformation").AsOLEPropertiesContainer();
+
+                var companyProperty = co.Properties.First(prop => prop.PropertyName == "PIDDSI_COMPANY");
+                Assert.AreEqual("My Company\0", companyProperty.Value);
+
+                var formatProperty = co.Properties.First(prop => prop.PropertyName == "PIDDSI_PRESFORMAT");
+                Assert.AreEqual("A4 Paper (210x297 mm)\0\0\0", formatProperty.Value);
+
+                var managerProperty = co.Properties.First(prop => prop.PropertyName == "PIDDSI_MANAGER");
+                Assert.AreEqual("The Boss\0", managerProperty.Value);
+            }
+        }
+
         [TestMethod]
         public void Test_SUMMARY_INFO_READ_UTF8_ISSUE_33()
         {
@@ -242,5 +291,47 @@ public void Test_SUMMARY_INFO_READ_LPWSTRING()
 
         }
 
+        // Test that we can modify an LPWSTR property, and the value is null terminated as required
+        [TestMethod]
+        public void Test_SUMMARY_INFO_MODIFY_LPWSTRING()
+        {
+            if (File.Exists("test_write_lpwstr.doc"))
+                File.Delete("test_write_lpwstr.doc");
+
+            // Modify some LPWSTR properties, and save to a new file
+            using (CompoundFile cf = new CompoundFile("wstr_presets.doc"))
+            {
+                var dsiStream = cf.RootStorage.GetStream("\u0005SummaryInformation");
+                var co = dsiStream.AsOLEPropertiesContainer();
+
+                var authorProperty = co.Properties.First(prop => prop.PropertyName == "PIDSI_AUTHOR");
+                Assert.AreEqual(VTPropertyType.VT_LPWSTR, authorProperty.VTType);
+                Assert.AreEqual("zkyiqpqoroxnbdwhnjfqroxlgylpbgcwuhjfifpkvycugvuecoputqgknnbs\0", authorProperty.Value);
+
+                var keyWordsProperty = co.Properties.First(prop => prop.PropertyName == "PIDSI_KEYWORDS");
+                Assert.AreEqual(VTPropertyType.VT_LPWSTR, keyWordsProperty.VTType);
+                Assert.AreEqual("abcdefghijk\0", keyWordsProperty.Value);
+
+                authorProperty.Value = "ABC";
+                keyWordsProperty.Value = "";
+                co.Save(dsiStream);
+                cf.SaveAs("test_write_lpwstr.doc");
+            }
+
+            // Open the new file and check for the expected values
+            using (CompoundFile cf = new CompoundFile("test_write_lpwstr.doc"))
+            {
+                var co = cf.RootStorage.GetStream("\u0005SummaryInformation").AsOLEPropertiesContainer();
+
+                var authorProperty = co.Properties.First(prop => prop.PropertyName == "PIDSI_AUTHOR");
+                Assert.AreEqual(VTPropertyType.VT_LPWSTR, authorProperty.VTType);
+                Assert.AreEqual("ABC\0", authorProperty.Value);
+
+                var keyWordsProperty = co.Properties.First(prop => prop.PropertyName == "PIDSI_KEYWORDS");
+                Assert.AreEqual(VTPropertyType.VT_LPWSTR, keyWordsProperty.VTType);
+                Assert.AreEqual("\0", keyWordsProperty.Value);
+            }
+        }
+
     }
 }