GitHub #186: Remove signatures from JAR files.

9 files changed, 356 insertions, 2 deletions

diff --git a/org.jacoco.ant.test/src/org/jacoco/ant/InstrumentTaskTest.xml b/org.jacoco.ant.test/src/org/jacoco/ant/InstrumentTaskTest.xml
index 9a9624c0..dbf616d1 100644
--- a/org.jacoco.ant.test/src/org/jacoco/ant/InstrumentTaskTest.xml
+++ b/org.jacoco.ant.test/src/org/jacoco/ant/InstrumentTaskTest.xml
@@ -50,6 +50,48 @@
 		</jacoco:instrument>
 	</target>
 	
+	<target name="testInstrumentRemoveSignatures">
+		<property name="lib.dir" location="${temp.dir}/lib"/>
+		<property name="instr.dir" location="${temp.dir}/instr"/>
+		<mkdir dir="${lib.dir}"/>
+		<mkdir dir="${instr.dir}"/>
+		
+		<jar destfile="${lib.dir}/test.jar">
+			<fileset dir="${org.jacoco.ant.instrumentTaskTest.classes.dir}" includes="**/*.class"/>
+		</jar>
+		<signjar jar="${lib.dir}/test.jar" keystore="${basedir}/data/keystore.jks" alias="test" storepass="password"/>
+		
+		<jacoco:instrument destdir="${instr.dir}">
+			<fileset dir="${lib.dir}" includes="*.jar"/>
+		</jacoco:instrument>
+		<au:assertLogContains text="Instrumented 12 classes to ${temp.dir}"/>
+
+		<unzip src="${instr.dir}/test.jar" dest="${instr.dir}"/>
+		<au:assertFileDoesntExist file="${instr.dir}/META-INF/TEST.RSA" />
+		<au:assertFileDoesntExist file="${instr.dir}/META-INF/TEST.SF" />
+	</target>	
+
+	<target name="testInstrumentKeepSignatures">
+		<property name="lib.dir" location="${temp.dir}/lib"/>
+		<property name="instr.dir" location="${temp.dir}/instr"/>
+		<mkdir dir="${lib.dir}"/>
+		<mkdir dir="${instr.dir}"/>
+		
+		<jar destfile="${lib.dir}/test.jar">
+			<fileset dir="${org.jacoco.ant.instrumentTaskTest.classes.dir}" includes="**/*.class"/>
+		</jar>
+		<signjar jar="${lib.dir}/test.jar" keystore="${basedir}/data/keystore.jks" alias="test" storepass="password"/>
+		
+		<jacoco:instrument destdir="${instr.dir}" removesignatures="false">
+			<fileset dir="${lib.dir}" includes="*.jar"/>
+		</jacoco:instrument>
+		<au:assertLogContains text="Instrumented 12 classes to ${temp.dir}"/>
+
+		<unzip src="${instr.dir}/test.jar" dest="${instr.dir}"/>
+		<au:assertFileExists file="${instr.dir}/META-INF/TEST.RSA" />
+		<au:assertFileExists file="${instr.dir}/META-INF/TEST.SF" />
+	</target>	
+	
 	<target name="testInstrumentAndRunWithConfigFile">
 		<jacoco:instrument destdir="${temp.dir}">
 			<fileset dir="${org.jacoco.ant.instrumentTaskTest.classes.dir}" includes="**/*.class"/>
diff --git a/org.jacoco.ant.test/src/org/jacoco/ant/data/keystore.jks b/org.jacoco.ant.test/src/org/jacoco/ant/data/keystore.jks
new file mode 100644
index 00000000..15545306
--- /dev/null
+++ b/org.jacoco.ant.test/src/org/jacoco/ant/data/keystore.jks
diff --git a/org.jacoco.ant/src/org/jacoco/ant/InstrumentTask.java b/org.jacoco.ant/src/org/jacoco/ant/InstrumentTask.java
index 98e9d4c0..18cc01cc 100644
--- a/org.jacoco.ant/src/org/jacoco/ant/InstrumentTask.java
+++ b/org.jacoco.ant/src/org/jacoco/ant/InstrumentTask.java
@@ -37,6 +37,8 @@ public class InstrumentTask extends Task {
 
 	private final Union files = new Union();
 
+	private boolean removesignatures = true;
+
 	/**
 	 * Sets the location of the instrumented classes.
 	 * 
@@ -48,6 +50,16 @@ public class InstrumentTask extends Task {
 	}
 
 	/**
+	 * Sets whether signatures should be removed from JAR files.
+	 * 
+	 * @param removesignatures
+	 *            <code>true</code> if signatures should be removed
+	 */
+	public void setRemovesignatures(final boolean removesignatures) {
+		this.removesignatures = removesignatures;
+	}
+
+	/**
 	 * This task accepts any number of class file resources.
 	 * 
 	 * @param resources
@@ -66,6 +78,7 @@ public class InstrumentTask extends Task {
 		int total = 0;
 		final Instrumenter instrumenter = new Instrumenter(
 				new OfflineInstrumentationAccessGenerator());
+		instrumenter.setRemoveSignatures(removesignatures);
 		final Iterator<?> resourceIterator = files.iterator();
 		while (resourceIterator.hasNext()) {
 			final Resource resource = (Resource) resourceIterator.next();
diff --git a/org.jacoco.core.test/src/org/jacoco/core/instr/InstrumenterTest.java b/org.jacoco.core.test/src/org/jacoco/core/instr/InstrumenterTest.java
index 00f8d84c..c9303d5a 100644
--- a/org.jacoco.core.test/src/org/jacoco/core/instr/InstrumenterTest.java
+++ b/org.jacoco.core.test/src/org/jacoco/core/instr/InstrumenterTest.java
@@ -234,4 +234,42 @@ public class InstrumenterTest {
 		assertEquals("text", new String(out.toByteArray()));
 	}
 
+	@Test
+	public void testInstrumentAll_RemoveSignatures() throws IOException {
+		ByteArrayOutputStream buffer = new ByteArrayOutputStream();
+		ZipOutputStream zipout = new ZipOutputStream(buffer);
+		zipout.putNextEntry(new ZipEntry("META-INF/MANIFEST.MF"));
+		zipout.putNextEntry(new ZipEntry("META-INF/ALIAS.SF"));
+		zipout.finish();
+		ByteArrayOutputStream out = new ByteArrayOutputStream();
+
+		int count = instrumenter.instrumentAll(
+				new ByteArrayInputStream(buffer.toByteArray()), out, "Test");
+
+		assertEquals(0, count);
+		ZipInputStream zipin = new ZipInputStream(new ByteArrayInputStream(
+				out.toByteArray()));
+		assertEquals("META-INF/MANIFEST.MF", zipin.getNextEntry().getName());
+		assertNull(zipin.getNextEntry());
+	}
+
+	@Test
+	public void testInstrumentAll_KeepSignatures() throws IOException {
+		ByteArrayOutputStream buffer = new ByteArrayOutputStream();
+		ZipOutputStream zipout = new ZipOutputStream(buffer);
+		zipout.putNextEntry(new ZipEntry("META-INF/ALIAS.SF"));
+		zipout.finish();
+		ByteArrayOutputStream out = new ByteArrayOutputStream();
+
+		instrumenter.setRemoveSignatures(false);
+		int count = instrumenter.instrumentAll(
+				new ByteArrayInputStream(buffer.toByteArray()), out, "Test");
+
+		assertEquals(0, count);
+		ZipInputStream zipin = new ZipInputStream(new ByteArrayInputStream(
+				out.toByteArray()));
+		assertEquals("META-INF/ALIAS.SF", zipin.getNextEntry().getName());
+		assertNull(zipin.getNextEntry());
+	}
+
 }
diff --git a/org.jacoco.core.test/src/org/jacoco/core/internal/instr/SignatureRemoverTest.java b/org.jacoco.core.test/src/org/jacoco/core/internal/instr/SignatureRemoverTest.java
new file mode 100644
index 00000000..31119958
--- /dev/null
+++ b/org.jacoco.core.test/src/org/jacoco/core/internal/instr/SignatureRemoverTest.java
@@ -0,0 +1,116 @@
+/*******************************************************************************
+ * Copyright (c) 2009, 2014 Mountainminds GmbH & Co. KG and Contributors
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ *
+ * Contributors:
+ *    Marc R. Hoffmann - initial API and implementation
+ *    
+ *******************************************************************************/
+package org.jacoco.core.internal.instr;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.InputStream;
+
+import org.junit.Before;
+import org.junit.Test;
+
+/**
+ * Unit tests for {@link SignatureRemover}.
+ */
+public class SignatureRemoverTest {
+
+	private SignatureRemover remover;
+
+	@Before
+	public void setup() {
+		remover = new SignatureRemover();
+	}
+
+	@Test
+	public void testRemoveNegative1() {
+		assertFalse(remover.removeEntry("META-INF/ALIAS.MF"));
+	}
+
+	@Test
+	public void testRemoveNegative2() {
+		assertFalse(remover.removeEntry("META-INF/sub/ALIAS.SF"));
+	}
+
+	@Test
+	public void testRemoveNegative3() {
+		remover.setActive(false);
+		assertFalse(remover.removeEntry("META-INF/SIG-ALIAS"));
+	}
+
+	@Test
+	public void testRemovePositive1() {
+		assertTrue(remover.removeEntry("META-INF/ALIAS.SF"));
+	}
+
+	@Test
+	public void testRemovePositive2() {
+		assertTrue(remover.removeEntry("META-INF/ALIAS.RSA"));
+	}
+
+	@Test
+	public void testRemovePositive3() {
+		assertTrue(remover.removeEntry("META-INF/ALIAS.DSA"));
+	}
+
+	@Test
+	public void testRemovePositive4() {
+		assertTrue(remover.removeEntry("META-INF/SIG-ALIAS"));
+	}
+
+	@Test
+	public void testFilterNegative1() throws IOException {
+		assertFalse(remover.filterEntry("MANIFEST.MF", null, null));
+	}
+
+	@Test
+	public void testFilterNegative2() throws IOException {
+		remover.setActive(false);
+		assertFalse(remover.filterEntry("META-INF/MANIFEST.MF", null, null));
+	}
+
+	@Test
+	public void testFilterPositive1() throws IOException {
+		String original = "Manifest-Version: 1.0\r\n"
+				+ "Created-By: Apache Maven\r\n" //
+				+ "Bundle-SymbolicName: org.jacoco.core\r\n" //
+				+ "\r\n"//
+				+ "Name: org/jacoco/example/A.class\r\n" //
+				+ "SHA1-Digest: z1ly8OewPb9LOCpfNaIAhEgXZ5I=\r\n" //
+				+ "\r\n" //
+				+ "Name: org/jacoco/example/B.class\r\n" //
+				+ "SHA1-Digest: nfE4+Vmekj0pE5z0m0frpb10Gl0=\r\n" //
+				+ "OtherInfo: keep this\r\n" //
+				+ "\r\n" //
+				+ "Name: org/jacoco/example/C.class\r\n" //
+				+ "SHA1-Digest: xaNEXNWCrlTVcqPrXL0TwTcsvXU=\r\n";
+		InputStream in = new ByteArrayInputStream(
+				original.getBytes("ISO-8859-1"));
+		ByteArrayOutputStream out = new ByteArrayOutputStream();
+
+		assertTrue(remover.filterEntry("META-INF/MANIFEST.MF", in, out));
+
+		String expected = "Manifest-Version: 1.0\r\n"
+				+ "Created-By: Apache Maven\r\n" //
+				+ "Bundle-SymbolicName: org.jacoco.core\r\n" //
+				+ "\r\n"//
+				+ "Name: org/jacoco/example/B.class\r\n" //
+				+ "OtherInfo: keep this\r\n" //
+				+ "\r\n";
+		assertEquals(expected, out.toString("ISO-8859-1"));
+	}
+
+}
diff --git a/org.jacoco.core/src/org/jacoco/core/instr/Instrumenter.java b/org.jacoco.core/src/org/jacoco/core/instr/Instrumenter.java
index 0ed55212..6d9c59f9 100644
--- a/org.jacoco.core/src/org/jacoco/core/instr/Instrumenter.java
+++ b/org.jacoco.core/src/org/jacoco/core/instr/Instrumenter.java
@@ -26,6 +26,7 @@ import org.jacoco.core.internal.Pack200Streams;
 import org.jacoco.core.internal.data.CRC64;
 import org.jacoco.core.internal.flow.ClassProbesAdapter;
 import org.jacoco.core.internal.instr.ClassInstrumenter;
+import org.jacoco.core.internal.instr.SignatureRemover;
 import org.jacoco.core.runtime.IExecutionDataAccessorGenerator;
 import org.objectweb.asm.ClassReader;
 import org.objectweb.asm.ClassVisitor;
@@ -38,6 +39,8 @@ public class Instrumenter {
 
 	private final IExecutionDataAccessorGenerator accessGenerator;
 
+	private final SignatureRemover signatureRemover;
+
 	/**
 	 * Creates a new instance based on the given runtime.
 	 * 
@@ -46,6 +49,20 @@ public class Instrumenter {
 	 */
 	public Instrumenter(final IExecutionDataAccessorGenerator runtime) {
 		this.accessGenerator = runtime;
+		this.signatureRemover = new SignatureRemover();
+	}
+
+	/**
+	 * Determines whether signatures should be removed from JAR files. This is
+	 * typically necessary as instrumentation modifies the class files and
+	 * therefore invalidates existing JAR signatures. Default is
+	 * <code>true</code>.
+	 * 
+	 * @param flag
+	 *            <code>true</code> if signatures should be removed
+	 */
+	public void setRemoveSignatures(final boolean flag) {
+		signatureRemover.setActive(flag);
 	}
 
 	/**
@@ -192,8 +209,15 @@ public class Instrumenter {
 		ZipEntry entry;
 		int count = 0;
 		while ((entry = zipin.getNextEntry()) != null) {
-			zipout.putNextEntry(new ZipEntry(entry.getName()));
-			count += instrumentAll(zipin, zipout, name + "@" + entry.getName());
+			final String entryName = entry.getName();
+			if (signatureRemover.removeEntry(entryName)) {
+				continue;
+			}
+
+			zipout.putNextEntry(new ZipEntry(entryName));
+			if (!signatureRemover.filterEntry(entryName, zipin, zipout)) {
+				count += instrumentAll(zipin, zipout, name + "@" + entryName);
+			}
 			zipout.closeEntry();
 		}
 		zipout.finish();
diff --git a/org.jacoco.core/src/org/jacoco/core/internal/instr/SignatureRemover.java b/org.jacoco.core/src/org/jacoco/core/internal/instr/SignatureRemover.java
new file mode 100644
index 00000000..2f9bac2a
--- /dev/null
+++ b/org.jacoco.core/src/org/jacoco/core/internal/instr/SignatureRemover.java
@@ -0,0 +1,112 @@
+/*******************************************************************************
+ * Copyright (c) 2009, 2014 Mountainminds GmbH & Co. KG and Contributors
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ *
+ * Contributors:
+ *    Marc R. Hoffmann - initial API and implementation
+ *    
+ *******************************************************************************/
+package org.jacoco.core.internal.instr;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.util.Collection;
+import java.util.Iterator;
+import java.util.jar.Attributes;
+import java.util.jar.Manifest;
+import java.util.regex.Pattern;
+
+/**
+ * Support class to filter entries from JARs related to signatures.
+ */
+public class SignatureRemover {
+
+	private static final Pattern SIGNATURE_FILES = Pattern
+			.compile("META-INF/[^/]*\\.SF|" //
+					+ "META-INF/[^/]*\\.DSA|" //
+					+ "META-INF/[^/]*\\.RSA|" //
+					+ "META-INF/SIG-[^/]*");
+
+	private static final String MANIFEST_MF = "META-INF/MANIFEST.MF";
+
+	private static final String DIGEST_SUFFIX = "-Digest";
+
+	private boolean active;
+
+	/**
+	 * Creates a new remover which is active.
+	 */
+	public SignatureRemover() {
+		active = true;
+	}
+
+	/**
+	 * Defines whether this remover should be active. If it is not active it
+	 * will not remove any entries.
+	 * 
+	 * @param active
+	 *            <code>true</code> if it should remove signature related
+	 *            entries.
+	 */
+	public void setActive(final boolean active) {
+		this.active = active;
+	}
+
+	/**
+	 * Checks whether a entry with the provided name should be ignored at all.
+	 * 
+	 * @param name
+	 *            path name of the entry in question
+	 * @return true is the entry should be ignored
+	 */
+	public boolean removeEntry(final String name) {
+		return active && SIGNATURE_FILES.matcher(name).matches();
+	}
+
+	/**
+	 * Filters the content of the entry with the provided name if necessary.
+	 * 
+	 * @param name
+	 *            path name of the entry in question
+	 * @param in
+	 *            source for the element to filter
+	 * @param out
+	 *            output for the filtered contents
+	 * @return <code>true</code> if the content was filtered
+	 * @throws IOException
+	 *             if the content can't be read or written
+	 */
+	public boolean filterEntry(final String name, final InputStream in,
+			final OutputStream out) throws IOException {
+		if (!active || !MANIFEST_MF.equals(name)) {
+			return false;
+		}
+		final Manifest mf = new Manifest(in);
+		filterManifestEntry(mf.getEntries().values());
+		mf.write(out);
+		return true;
+	}
+
+	private void filterManifestEntry(final Collection<Attributes> entry) {
+		for (final Iterator<Attributes> i = entry.iterator(); i.hasNext();) {
+			final Attributes attributes = i.next();
+			filterManifestEntryAttributes(attributes);
+			if (attributes.isEmpty()) {
+				i.remove();
+			}
+		}
+	}
+
+	private void filterManifestEntryAttributes(final Attributes attrs) {
+		for (final Iterator<Object> i = attrs.keySet().iterator(); i.hasNext();) {
+			if (String.valueOf(i.next()).endsWith(DIGEST_SUFFIX)) {
+				i.remove();
+			}
+		}
+	}
+
+}
diff --git a/org.jacoco.doc/docroot/doc/ant.html b/org.jacoco.doc/docroot/doc/ant.html
index 553c53e7..7400b7d5 100644
--- a/org.jacoco.doc/docroot/doc/ant.html
+++ b/org.jacoco.doc/docroot/doc/ant.html
@@ -892,6 +892,13 @@
       <td>Directory location to write the instrumented files to.</td>
       <td><i>none (required)</i></td>
     </tr>
+    <tr>
+      <td><code>removesignatures</code></td>
+      <td>If set to <code>true</code> all signature related information is 
+          stripped from JARs. This is typically necessary as instrumentation
+          breaks the signatures of the original class files.</td>
+      <td><code>true</code></td>
+    </tr>
   </tbody>
 </table>
 
diff --git a/org.jacoco.doc/docroot/doc/changes.html b/org.jacoco.doc/docroot/doc/changes.html
index 8873cd97..715057a2 100644
--- a/org.jacoco.doc/docroot/doc/changes.html
+++ b/org.jacoco.doc/docroot/doc/changes.html
@@ -24,6 +24,8 @@
 <ul>
   <li>Warnings are logged during report generation if different versions of
       classes are used than at runtime (GitHub <a href="https://github.com/jacoco/jacoco/issues/185">#185</a>).</li>
+  <li>Signatures are removed from instrumented JAR files
+      (GitHub <a href="https://github.com/jacoco/jacoco/issues/186">#186</a>).</li>
 </ul>
 
 <h3>Fixed Bugs</h3>