(GH-1047) Harden Xml serialization and deserialization #1084

RichiCoder1 · 2016-12-10T04:36:07Z

Rewrites the XML serialization service in a few specific ways:

Should update hash checks now builds the update file in memory rather than committing it the disk, and checks the hash in memory.
The ".update" file now using the slightly hardier File Replace to replace the undated file, and also generates a ".backup".
When deserializing, we now check for a ".backup" if serialization fails and attempt that too.
Generally use streams and dispose where possible.

RichiCoder1 · 2016-12-10T04:43:22Z

I'm thinking I may add some unit tests around this too, but I have nothing hard group just yet

ferventcoder · 2016-12-16T16:54:46Z

src/chocolatey/infrastructure/adapters/HashAlgorithm.cs

+        public byte[] ComputeHash(Stream stream)
+        {
+            return _algorithm.ComputeHash(stream);
+        }


ferventcoder · 2016-12-16T16:55:33Z

src/chocolatey/infrastructure/cryptography/CryptoHashProvider.cs

@@ -1,4 +1,4 @@
-// Copyright � 2011 - Present RealDimensions Software, LLC
+// Copyright � 2011 - Present RealDimensions Software, LLC


Looks like you dropped this from UTF-8. Very much dislike when Visual Studio does this. 👎

Darn't. Did even notice that.

Will fix both occurences

ferventcoder

Looking great so far. I am concerned that the files did change from UTF8 (no BOM) to ANSI. If you could fix those files that were changed by VS and squash that back into this, that would be good.

ferventcoder · 2016-12-16T16:56:02Z

src/chocolatey/infrastructure/cryptography/IHashProvider.cs

@@ -1,4 +1,4 @@
-// Copyright � 2011 - Present RealDimensions Software, LLC
+// Copyright � 2011 - Present RealDimensions Software, LLC


Same here :/

ferventcoder · 2016-12-16T16:58:19Z

src/chocolatey/infrastructure/services/XmlService.cs

+                            } 
+                            throw;
+                        }
+                    }


So this is the meat - just compares the stream versus the file before it creates the old file? This would be great in low access environments so there is no error message when it can't create the file.

Was exactly the idea! No more Failed to write chocolatey.config.update permission errors :D

ferventcoder · 2016-12-16T17:03:57Z

Minor nitpicks to your git commit:

You don't need the colon in the commit summary (GH-1047): Harden Xml serialization and deserialization
misspelled deserializing in third bullet point - When desieralizing,

The other change I would request is to keep the same (GH-1047) to prepend, but move the changes to HashAlgorithm/HashProvider and FileSystem to their own commits. Those changes stand on their own. So I'd hope for three commits here instead of one.

ferventcoder · 2017-01-21T23:37:57Z

src/chocolatey/infrastructure/services/XmlService.cs

+                                memoryStream.CopyTo(updateFileStream);
+                            }
+                            _fileSystem.replace_file(tempUpdateFile, xmlFilePath, xmlFilePath + ".backup");
+                        }


Notes for me. We should probably delete the backup file when things are successful.

I mostly keep it because it allows us to recover if a future operation fails for some reason.

ferventcoder · 2017-01-21T23:38:39Z

src/chocolatey/infrastructure/services/XmlService.cs

-                    textWriter.Flush();
+                    // Write the updated file to memory
+                    using(var memoryStream = new MemoryStream())                        
+                    using(var textWriter = new StreamWriter(memoryStream, encoding: new UTF8Encoding(encoderShouldEmitUTF8Identifier: true)))


minor nit - textWriter is probably now streamWriter

Also instead of using, I actually prefer to be explicit when it comes to doing file writes.

I'm not sure what you mean with that second comment? Neither stream is personally involved in writing the file to disk.

Allows for the more correct replacement and backup of files on the filesystem.

…on disk.

RichiCoder1 · 2017-02-08T19:52:01Z

Aside from your using request, which I couldn't parse, this should be good to go. Sorry for sitting on this for so long.

ferventcoder · 2017-03-23T01:08:31Z

Merged into stable at 5134b46. Thanks for the contributions!! This is going to add quite a bit in what it provides!

RichiCoder1 force-pushed the feature/harden_config_serialization branch from c0badd5 to 922af62 Compare December 10, 2016 04:42

ferventcoder reviewed Dec 16, 2016

View reviewed changes

ferventcoder requested changes Dec 16, 2016

View reviewed changes

ferventcoder assigned ferventcoder and RichiCoder1 Dec 16, 2016

ferventcoder added the 0 - Waiting on User label Dec 16, 2016

RichiCoder1 force-pushed the feature/harden_config_serialization branch 2 times, most recently from 6502951 to 63cdb82 Compare December 20, 2016 12:03

RichiCoder1 mentioned this pull request Dec 20, 2016

Error starting v0.14.0-unstable0147 chocolatey/ChocolateyGUI#360

Closed

ferventcoder reviewed Jan 21, 2017

View reviewed changes

RichiCoder1 added 3 commits February 8, 2017 13:33

(chocolateyGH-1047) Add additional hash stream options to Providers

04b39ca

(chocolateyGH-1047) Add "replace" filesystem operation

707e90c

Allows for the more correct replacement and backup of files on the filesystem.

(chocolateyGH-1047) Make xml serialization more robust and efficient.

9d6a366

RichiCoder1 force-pushed the feature/harden_config_serialization branch from 63cdb82 to ab4af16 Compare February 8, 2017 19:33

RichiCoder1 changed the title ~~(GH-1047): Harden Xml serialization and deserialization~~ (GH-1047) Harden Xml serialization and deserialization Feb 8, 2017

(chocolateyGH-1047) Account for cases where there's no original file …

ce4181f

…on disk.

RichiCoder1 force-pushed the feature/harden_config_serialization branch from ab4af16 to ce4181f Compare February 8, 2017 19:48

ferventcoder closed this Mar 23, 2017

ferventcoder reopened this Mar 23, 2017

ferventcoder approved these changes Mar 23, 2017

View reviewed changes

ferventcoder closed this Mar 23, 2017

RichiCoder1 deleted the feature/harden_config_serialization branch March 23, 2017 15:29

RichiCoder1 restored the feature/harden_config_serialization branch March 23, 2017 15:29

RichiCoder1 deleted the feature/harden_config_serialization branch March 23, 2017 19:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

(GH-1047) Harden Xml serialization and deserialization #1084

(GH-1047) Harden Xml serialization and deserialization #1084

RichiCoder1 commented Dec 10, 2016 •

edited by ferventcoder

Loading

RichiCoder1 commented Dec 10, 2016

ferventcoder Dec 16, 2016

ferventcoder Dec 16, 2016

RichiCoder1 Dec 16, 2016

RichiCoder1 Dec 16, 2016

ferventcoder left a comment

ferventcoder Dec 16, 2016

ferventcoder Dec 16, 2016

RichiCoder1 Dec 16, 2016

ferventcoder commented Dec 16, 2016 •

edited

Loading

ferventcoder Jan 21, 2017

RichiCoder1 Feb 8, 2017

ferventcoder Jan 21, 2017

ferventcoder Jan 21, 2017 •

edited

Loading

RichiCoder1 Feb 8, 2017

RichiCoder1 commented Feb 8, 2017

ferventcoder commented Mar 23, 2017

		@@ -1,4 +1,4 @@
		// Copyright � 2011 - Present RealDimensions Software, LLC
		// Copyright � 2011 - Present RealDimensions Software, LLC

(GH-1047) Harden Xml serialization and deserialization #1084

(GH-1047) Harden Xml serialization and deserialization #1084

Conversation

RichiCoder1 commented Dec 10, 2016 • edited by ferventcoder Loading

RichiCoder1 commented Dec 10, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ferventcoder left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ferventcoder commented Dec 16, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ferventcoder Jan 21, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RichiCoder1 commented Feb 8, 2017

ferventcoder commented Mar 23, 2017

RichiCoder1 commented Dec 10, 2016 •

edited by ferventcoder

Loading

ferventcoder commented Dec 16, 2016 •

edited

Loading

ferventcoder Jan 21, 2017 •

edited

Loading