From 89a11462c7e77484e8e9fff36baa404eaba9c8e8 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Mon, 9 Sep 2013 10:06:55 -0400 Subject: [PATCH] [FIXED JENKINS-18337] Must use xmlEscape for freeform text fields in fingerprint XML. --- changelog.html | 3 ++ .../main/java/hudson/model/Fingerprint.java | 31 +++++++++++++++---- .../model/FingerprintCleanupThread.java | 2 +- .../java/hudson/model/FingerprintTest.java | 3 +- 4 files changed, 31 insertions(+), 8 deletions(-) diff --git a/changelog.html b/changelog.html index 2c290ebd18..a4af17ce93 100644 --- a/changelog.html +++ b/changelog.html @@ -64,6 +64,9 @@ Upcoming changes
  • Identify user agent for Internet Explorer 11. (issue 19171) +
  • + Since 1.518, fingerprint serialization broke when job or file names contained XML special characters like ampersands. + (issue 18337)
  • JavaScript error in the checkUrl computation shouldn't break the job configuration page. (issue 19457) diff --git a/core/src/main/java/hudson/model/Fingerprint.java b/core/src/main/java/hudson/model/Fingerprint.java index 4de1899d00..59a759460e 100644 --- a/core/src/main/java/hudson/model/Fingerprint.java +++ b/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(" "); w.print(" "); - w.print(original.name); + w.print(Util.xmlEscape(original.name)); w.println(""); w.print(" "); w.print(original.number); @@ -1120,13 +1122,13 @@ public class Fingerprint implements ModelObject, Saveable { w.print(Util.toHexString(md5sum)); w.println(""); w.print(" "); - w.print(fileName); + w.print(Util.xmlEscape(fileName)); w.println(""); w.println(" "); for (Map.Entry e : usages.entrySet()) { w.println(" "); w.print(" "); - w.print(e.getKey()); + w.print(Util.xmlEscape(e.getKey())); w.println(""); w.print(" "); 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(usages) + ",facets=" + facets + "]"; diff --git a/core/src/main/java/hudson/model/FingerprintCleanupThread.java b/core/src/main/java/hudson/model/FingerprintCleanupThread.java index 1b0dc37278..f8be385ba0 100644 --- a/core/src/main/java/hudson/model/FingerprintCleanupThread.java +++ b/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 { diff --git a/core/src/test/java/hudson/model/FingerprintTest.java b/core/src/test/java/hudson/model/FingerprintTest.java index da1ef4e9fb..fa77da0364 100644 --- a/core/src/test/java/hudson/model/FingerprintTest.java +++ b/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")); -- GitLab