[FIXED JENKINS-18337] Must use xmlEscape for freeform text fields in fingerprint XML.

changelog.html

@@ -64,6 +64,9 @@ Upcoming changes</a>
   <li class=bug>
     Identify user agent for Internet Explorer 11.
     (<a href="https://issues.jenkins-ci.org/browse/JENKINS-19171">issue 19171</a>)
+  <li class=bug>
+    Since 1.518, fingerprint serialization broke when job or file names contained XML special characters like ampersands.
+    (<a href="https://issues.jenkins-ci.org/browse/JENKINS-18337">issue 18337</a>)
   <li class=rfe>
     JavaScript error in the checkUrl computation shouldn't break the job configuration page.
     (<a href="https://issues.jenkins-ci.org/browse/JENKINS-19457">issue 19457</a>)

core/src/main/java/hudson/model/Fingerprint.java

@@ -66,6 +66,8 @@ import java.util.Map.Entry;
 import java.util.TreeMap;
 import java.util.logging.Level;
 import java.util.logging.Logger;
+import javax.annotation.CheckForNull;
+import org.xmlpull.v1.XmlPullParserException;
 
 /**
  * A file being tracked by Jenkins.
@@ -1109,7 +1111,7 @@ public class Fingerprint implements ModelObject, Saveable {
                 if (original != null) {
                     w.println("  <original>");
                     w.print("    <name>");
-                    w.print(original.name);
+                    w.print(Util.xmlEscape(original.name));
                     w.println("</name>");
                     w.print("    <number>");
                     w.print(original.number);
@@ -1120,13 +1122,13 @@ public class Fingerprint implements ModelObject, Saveable {
                 w.print(Util.toHexString(md5sum));
                 w.println("</md5sum>");
                 w.print("  <fileName>");
-                w.print(fileName);
+                w.print(Util.xmlEscape(fileName));
                 w.println("</fileName>");
                 w.println("  <usages>");
                 for (Map.Entry<String,RangeSet> e : usages.entrySet()) {
                     w.println("    <entry>");
                     w.print("      <string>");
-                    w.print(e.getKey());
+                    w.print(Util.xmlEscape(e.getKey()));
                     w.println("</string>");
                     w.print("      <ranges>");
                     w.print(RangeSet.ConverterImpl.serialize(e.getValue()));
@@ -1195,10 +1197,10 @@ public class Fingerprint implements ModelObject, Saveable {
     /**
      * Loads a {@link Fingerprint} from a file in the image.
      */
-    /*package*/ static Fingerprint load(byte[] md5sum) throws IOException {
+    /*package*/ static @CheckForNull Fingerprint load(byte[] md5sum) throws IOException {
         return load(getFingerprintFile(md5sum));
     }
-    /*package*/ static Fingerprint load(File file) throws IOException {
+    /*package*/ static @CheckForNull Fingerprint load(File file) throws IOException {
         XmlFile configFile = getConfigFile(file);
         if(!configFile.exists())
             return null;
@@ -1224,7 +1226,13 @@ public class Fingerprint implements ModelObject, Saveable {
                 // generally we don't want to wipe out user data just because we can't load it,
                 // but if the file size is 0, which is what's reported in HUDSON-2012, then it seems
                 // like recovering it silently by deleting the file is not a bad idea.
-                logger.log(Level.WARNING, "Size zero fingerprint. Disk corruption? "+configFile,e);
+                logger.log(Level.WARNING, "Size zero fingerprint. Disk corruption? {0}", configFile);
+                file.delete();
+                return null;
+            }
+            String parseError = messageOfXmlPullParserException(e);
+            if (parseError != null) {
+                logger.log(Level.WARNING, "Malformed XML in {0}: {1}", new Object[] {configFile, parseError});
                 file.delete();
                 return null;
             }
@@ -1232,6 +1240,17 @@ public class Fingerprint implements ModelObject, Saveable {
             throw e;
         }
     }
+    private static String messageOfXmlPullParserException(Throwable t) {
+        if (t instanceof XmlPullParserException) {
+            return t.getMessage();
+        }
+        Throwable t2 = t.getCause();
+        if (t2 != null) {
+            return messageOfXmlPullParserException(t2);
+        } else {
+            return null;
+        }
+    }
 
     @Override public String toString() {
         return "Fingerprint[original=" + original + ",hash=" + getHashString() + ",fileName=" + fileName + ",timestamp=" + DATE_CONVERTER.toString(timestamp) + ",usages=" + new TreeMap<String,RangeSet>(usages) + ",facets=" + facets + "]";

core/src/main/java/hudson/model/FingerprintCleanupThread.java

@@ -100,7 +100,7 @@ public final class FingerprintCleanupThread extends AsyncPeriodicWork {
     private boolean check(File fingerprintFile) {
         try {
             Fingerprint fp = Fingerprint.load(fingerprintFile);
-            if(!fp.isAlive()) {
+            if (fp == null || !fp.isAlive()) {
                 fingerprintFile.delete();
                 return true;
             } else {

core/src/test/java/hudson/model/FingerprintTest.java

@@ -220,7 +220,7 @@ public class FingerprintTest {
     }
 
     @Test public void roundTrip() throws Exception {
-        Fingerprint f = new Fingerprint(new Fingerprint.BuildPtr("foo", 13), "stuff.jar", SOME_MD5);
+        Fingerprint f = new Fingerprint(new Fingerprint.BuildPtr("foo", 13), "stuff&more.jar", SOME_MD5);
         f.addWithoutSaving("some", 1);
         f.addWithoutSaving("some", 2);
         f.addWithoutSaving("some", 3);
@@ -229,6 +229,7 @@ public class FingerprintTest {
         File xml = new File(new File(tmp.getRoot(), "dir"), "fp.xml");
         f.save(xml);
         Fingerprint f2 = Fingerprint.load(xml);
+        assertNotNull(f2);
         assertEquals(f.toString(), f2.toString());
         f.facets.setOwner(Saveable.NOOP);
         f.facets.add(new TestFacet(f, 123, "val"));