Merge branch 'master' into issue-525upstream-pull-525

5 files changed, 240 insertions, 34 deletions

diff --git a/org.jacoco.agent.rt.test/src/org/jacoco/agent/rt/internal/CoverageTransformerTest.java b/org.jacoco.agent.rt.test/src/org/jacoco/agent/rt/internal/CoverageTransformerTest.java
index 7ebe1222..6ad24dcf 100644
--- a/org.jacoco.agent.rt.test/src/org/jacoco/agent/rt/internal/CoverageTransformerTest.java
+++ b/org.jacoco.agent.rt.test/src/org/jacoco/agent/rt/internal/CoverageTransformerTest.java
@@ -212,11 +212,11 @@ public class CoverageTransformerTest {
 					protectionDomain, null);
 			fail("IllegalClassFormatException expected.");
 		} catch (IllegalClassFormatException e) {
-			assertEquals("Error while instrumenting class org.jacoco.Sample.",
+			assertEquals("Error while instrumenting org.jacoco.Sample.",
 					e.getMessage());
 		}
 		recorder.assertException(IllegalClassFormatException.class,
-				"Error while instrumenting class org.jacoco.Sample.",
+				"Error while instrumenting org.jacoco.Sample.",
 				IOException.class);
 		recorder.clear();
 	}
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 67073d7d..f674c666 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
@@ -23,6 +23,7 @@ import java.io.ObjectInputStream;
 import java.io.ObjectOutputStream;
 import java.io.OutputStream;
 import java.io.Serializable;
+import java.util.Arrays;
 import java.util.jar.JarInputStream;
 import java.util.jar.JarOutputStream;
 import java.util.jar.Pack200;
@@ -91,31 +92,57 @@ public class InstrumenterTest {
 		assertEquals("org.jacoco.core.instr.InstrumenterTest", clazz.getName());
 	}
 
+	/**
+	 * Triggers exception in {@link Instrumenter#instrument(byte[], String)}.
+	 */
 	@Test
 	public void testInstrumentBrokenClass1() throws IOException {
 		final byte[] brokenclass = TargetLoader
 				.getClassDataAsBytes(AnalyzerTest.class);
 		brokenclass[10] = 0x23;
 		try {
-			instrumenter.instrument(brokenclass, "Broken");
+			instrumenter.instrument(brokenclass, "Broken.class");
 			fail();
 		} catch (IOException e) {
-			assertEquals("Error while instrumenting class Broken.",
+			assertEquals("Error while instrumenting Broken.class.",
 					e.getMessage());
 		}
 	}
 
+	private static class BrokenInputStream extends InputStream {
+		@Override
+		public int read() throws IOException {
+			throw new IOException();
+		}
+	}
+
+	/**
+	 * Triggers exception in
+	 * {@link Instrumenter#instrument(InputStream, String)}.
+	 */
 	@Test
-	public void testInstrumentBrokenClass2() throws IOException {
-		final byte[] brokenclass = TargetLoader
-				.getClassDataAsBytes(AnalyzerTest.class);
-		brokenclass[10] = 0x23;
+	public void testInstrumentBrokenStream() {
 		try {
-			instrumenter.instrument(new ByteArrayInputStream(brokenclass),
-					"Broken");
-			fail();
+			instrumenter.instrument(new BrokenInputStream(), "BrokenStream");
+			fail("exception expected");
+		} catch (IOException e) {
+			assertEquals("Error while instrumenting BrokenStream.",
+					e.getMessage());
+		}
+	}
+
+	/**
+	 * Triggers exception in
+	 * {@link Instrumenter#instrument(InputStream, OutputStream, String)}.
+	 */
+	@Test
+	public void testInstrumentBrokenStream2() {
+		try {
+			instrumenter.instrument(new BrokenInputStream(),
+					new ByteArrayOutputStream(), "BrokenStream");
+			fail("exception expected");
 		} catch (IOException e) {
-			assertEquals("Error while instrumenting class Broken.",
+			assertEquals("Error while instrumenting BrokenStream.",
 					e.getMessage());
 		}
 	}
@@ -169,6 +196,97 @@ public class InstrumenterTest {
 		assertNull(zipin.getNextEntry());
 	}
 
+	/**
+	 * Triggers exception in
+	 * {@link org.jacoco.core.internal.ContentTypeDetector#ContentTypeDetector(InputStream)}.
+	 */
+	@Test
+	public void testInstrumentAll_Broken() {
+		try {
+			instrumenter.instrumentAll(new BrokenInputStream(),
+					new ByteArrayOutputStream(), "Broken");
+			fail("exception expected");
+		} catch (IOException e) {
+			assertEquals("Error while instrumenting Broken.", e.getMessage());
+		}
+	}
+
+	/**
+	 * Triggers exception in
+	 * {@link Instrumenter#copy(InputStream, OutputStream)}.
+	 */
+	@Test
+	public void testInstrumentAll_Broken2() {
+		final InputStream inputStream = new InputStream() {
+			private int count;
+
+			@Override
+			public int read() throws IOException {
+				count++;
+				if (count > 4) {
+					throw new IOException();
+				}
+				return 0;
+			}
+		};
+
+		try {
+			instrumenter.instrumentAll(inputStream, new ByteArrayOutputStream(),
+					"Broken");
+		} catch (IOException e) {
+			assertEquals("Error while instrumenting Broken.", e.getMessage());
+		}
+	}
+
+	/**
+	 * Triggers exception in
+	 * {@link Instrumenter#nextEntry(ZipInputStream, String)}.
+	 */
+	@Test
+	public void testInstrumentAll_BrokenZip() {
+		final byte[] buffer = new byte[30];
+		buffer[0] = 0x50;
+		buffer[1] = 0x4b;
+		buffer[2] = 0x03;
+		buffer[3] = 0x04;
+		Arrays.fill(buffer, 4, buffer.length, (byte) 0x42);
+
+		try {
+			instrumenter.instrumentAll(new ByteArrayInputStream(buffer),
+					new ByteArrayOutputStream(), "Test.zip");
+			fail("exception expected");
+		} catch (IOException e) {
+			assertEquals("Error while instrumenting Test.zip.", e.getMessage());
+		}
+	}
+
+	/**
+	 * With JDK <= 6 triggers exception in
+	 * {@link Instrumenter#copy(InputStream, OutputStream)}.
+	 *
+	 * With JDK > 6 triggers exception in
+	 * {@link org.jacoco.core.internal.ContentTypeDetector#ContentTypeDetector(InputStream)}.
+	 */
+	@Test
+	public void testInstrumentAll_BrokenZipEntry() throws IOException {
+		ByteArrayOutputStream out = new ByteArrayOutputStream();
+		ZipOutputStream zip = new ZipOutputStream(out);
+		zip.putNextEntry(new ZipEntry("brokenentry.txt"));
+		out.write(0x23); // Unexpected data here
+		zip.close();
+
+		try {
+			instrumenter.instrumentAll(
+					new ByteArrayInputStream(out.toByteArray()),
+					new ByteArrayOutputStream(), "broken.zip");
+			fail("exception expected");
+		} catch (IOException e) {
+			assertEquals(
+					"Error while instrumenting broken.zip@brokenentry.txt.",
+					e.getMessage());
+		}
+	}
+
 	@Test
 	public void testInstrumentAll_BrokenClassFileInZip() throws IOException {
 		ByteArrayOutputStream buffer = new ByteArrayOutputStream();
@@ -186,12 +304,28 @@ public class InstrumenterTest {
 					"test.zip");
 			fail();
 		} catch (IOException e) {
-			assertEquals(
-					"Error while instrumenting class test.zip@Test.class.",
+			assertEquals("Error while instrumenting test.zip@Test.class.",
 					e.getMessage());
 		}
 	}
 
+	/**
+	 * Triggers exception in
+	 * {@link Instrumenter#instrumentGzip(InputStream, OutputStream, String)}.
+	 */
+	@Test
+	public void testInstrumentAll_BrokenGZ() {
+		final byte[] buffer = new byte[] { 0x1f, (byte) 0x8b, 0x00, 0x00 };
+
+		try {
+			instrumenter.instrumentAll(new ByteArrayInputStream(buffer),
+					new ByteArrayOutputStream(), "Test.gz");
+			fail("exception expected");
+		} catch (IOException e) {
+			assertEquals("Error while instrumenting Test.gz.", e.getMessage());
+		}
+	}
+
 	@Test
 	public void testInstrumentAll_Pack200() throws IOException {
 		ByteArrayOutputStream jarbuffer = new ByteArrayOutputStream();
@@ -223,6 +357,24 @@ public class InstrumenterTest {
 		assertNull(zipin.getNextEntry());
 	}
 
+	/**
+	 * Triggers exception in
+	 * {@link Instrumenter#instrumentPack200(InputStream, OutputStream, String)}.
+	 */
+	@Test
+	public void testInstrumentAll_BrokenPack200() {
+		final byte[] buffer = new byte[] { (byte) 0xca, (byte) 0xfe,
+				(byte) 0xd0, 0x0d };
+
+		try {
+			instrumenter.instrumentAll(new ByteArrayInputStream(buffer),
+					new ByteArrayOutputStream(), "Test.pack200");
+		} catch (IOException e) {
+			assertEquals("Error while instrumenting Test.pack200.",
+					e.getMessage());
+		}
+	}
+
 	@Test
 	public void testInstrumentAll_Other() throws IOException {
 		InputStream in = new ByteArrayInputStream("text".getBytes());
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 8b46907a..eb91f561 100644
--- a/org.jacoco.core/src/org/jacoco/core/instr/Instrumenter.java
+++ b/org.jacoco.core/src/org/jacoco/core/instr/Instrumenter.java
@@ -100,7 +100,7 @@ public class Instrumenter {
 	 *            a name used for exception messages
 	 * @return instrumented definition
 	 * @throws IOException
-	 *             if the class can't be analyzed
+	 *             if the class can't be instrumented
 	 */
 	public byte[] instrument(final byte[] buffer, final String name)
 			throws IOException {
@@ -132,11 +132,13 @@ public class Instrumenter {
 	 */
 	public byte[] instrument(final InputStream input, final String name)
 			throws IOException {
+		final byte[] bytes;
 		try {
-			return instrument(Java9Support.readFully(input), name);
-		} catch (final RuntimeException e) {
+			bytes = Java9Support.readFully(input);
+		} catch (final IOException e) {
 			throw instrumentError(name, e);
 		}
+		return instrument(bytes, name);
 	}
 
 	/**
@@ -154,17 +156,13 @@ public class Instrumenter {
 	 */
 	public void instrument(final InputStream input, final OutputStream output,
 			final String name) throws IOException {
-		try {
-			output.write(instrument(Java9Support.readFully(input), name));
-		} catch (final RuntimeException e) {
-			throw instrumentError(name, e);
-		}
+		output.write(instrument(input, name));
 	}
 
 	private IOException instrumentError(final String name,
-			final RuntimeException cause) {
-		final IOException ex = new IOException(String.format(
-				"Error while instrumenting class %s.", name));
+			final Exception cause) {
+		final IOException ex = new IOException(
+				String.format("Error while instrumenting %s.", name));
 		ex.initCause(cause);
 		return ex;
 	}
@@ -187,7 +185,12 @@ public class Instrumenter {
 	 */
 	public int instrumentAll(final InputStream input,
 			final OutputStream output, final String name) throws IOException {
-		final ContentTypeDetector detector = new ContentTypeDetector(input);
+		final ContentTypeDetector detector;
+		try {
+			detector = new ContentTypeDetector(input);
+		} catch (IOException e) {
+			throw instrumentError(name, e);
+		}
 		switch (detector.getType()) {
 		case ContentTypeDetector.CLASSFILE:
 			instrument(detector.getInputStream(), output, name);
@@ -199,7 +202,7 @@ public class Instrumenter {
 		case ContentTypeDetector.PACK200FILE:
 			return instrumentPack200(detector.getInputStream(), output, name);
 		default:
-			copy(detector.getInputStream(), output);
+			copy(detector.getInputStream(), output, name);
 			return 0;
 		}
 	}
@@ -210,7 +213,7 @@ public class Instrumenter {
 		final ZipOutputStream zipout = new ZipOutputStream(output);
 		ZipEntry entry;
 		int count = 0;
-		while ((entry = zipin.getNextEntry()) != null) {
+		while ((entry = nextEntry(zipin, name)) != null) {
 			final String entryName = entry.getName();
 			if (signatureRemover.removeEntry(entryName)) {
 				continue;
@@ -226,30 +229,59 @@ public class Instrumenter {
 		return count;
 	}
 
+	private ZipEntry nextEntry(ZipInputStream input, String location)
+			throws IOException {
+		try {
+			return input.getNextEntry();
+		} catch (IOException e) {
+			throw instrumentError(location, e);
+		}
+	}
+
 	private int instrumentGzip(final InputStream input,
 			final OutputStream output, final String name) throws IOException {
+		final GZIPInputStream gzipInputStream;
+		try {
+			gzipInputStream = new GZIPInputStream(input);
+		} catch (IOException e) {
+			throw instrumentError(name, e);
+		}
 		final GZIPOutputStream gzout = new GZIPOutputStream(output);
-		final int count = instrumentAll(new GZIPInputStream(input), gzout, name);
+		final int count = instrumentAll(gzipInputStream, gzout, name);
 		gzout.finish();
 		return count;
 	}
 
 	private int instrumentPack200(final InputStream input,
 			final OutputStream output, final String name) throws IOException {
+		final InputStream unpackedInput;
+		try {
+			unpackedInput = Pack200Streams.unpack(input);
+		} catch (IOException e) {
+			throw instrumentError(name, e);
+		}
 		final ByteArrayOutputStream buffer = new ByteArrayOutputStream();
-		final int count = instrumentAll(Pack200Streams.unpack(input), buffer,
-				name);
+		final int count = instrumentAll(unpackedInput, buffer, name);
 		Pack200Streams.pack(buffer.toByteArray(), output);
 		return count;
 	}
 
-	private void copy(final InputStream input, final OutputStream output)
-			throws IOException {
+	private void copy(final InputStream input, final OutputStream output,
+			final String name) throws IOException {
 		final byte[] buffer = new byte[1024];
 		int len;
-		while ((len = input.read(buffer)) != -1) {
+		while ((len = read(input, buffer, name)) != -1) {
 			output.write(buffer, 0, len);
 		}
 	}
 
+	private int read(final InputStream input, final byte[] buffer,
+			final String name) throws IOException {
+		try {
+			return input.read(buffer);
+		} catch (IOException e) {
+			throw instrumentError(name, e);
+		}
+	}
+
 }
diff --git a/org.jacoco.doc/docroot/doc/changes.html b/org.jacoco.doc/docroot/doc/changes.html
index 42aeb496..cb7b8557 100644
--- a/org.jacoco.doc/docroot/doc/changes.html
+++ b/org.jacoco.doc/docroot/doc/changes.html
@@ -40,6 +40,13 @@
       (GitHub <a href="https://github.com/jacoco/jacoco/issues/525">#525</a>).</li>
 </ul>
 
+<h3>Non-functional Changes</h3>
+<ul>
+  <li>More information about context is provided when unable to read input during
+      instrumentation
+      (GitHub <a href="https://github.com/jacoco/jacoco/issues/527">#527</a>).</li>
+</ul>
+
 <h2>Release 0.7.9 (2017/02/05)</h2>
 
 <h3>Fixed Bugs</h3>
diff --git a/org.jacoco.doc/docroot/doc/faq.html b/org.jacoco.doc/docroot/doc/faq.html
index 0efbce51..5c5275ab 100644
--- a/org.jacoco.doc/docroot/doc/faq.html
+++ b/org.jacoco.doc/docroot/doc/faq.html
@@ -149,6 +149,21 @@
   classpath and accessible from by the instrumented classes. 
 </p>
 
+<h3>Why do I get a <code>StackOverflowError</code> during code coverage analysis?</h3>
+<p>
+  There are two known reasons for this:
+</p>
+<ul>
+  <li>Misconfiguration: If you configure two JaCoCo agents of different releases
+      they will instrument each other and cause a endless recursion. Check the
+      effective java command line and avoid such configurations.</li>
+  <li>Heavy stack usage: JaCoCo instrumentation adds a small runtime overhead
+      by adding a local variable to each method. If your application is already
+      close to the maximum stack size this can eventually lead to an
+      <code>StackOverflowError</code>. Increase the maximum java stack size with
+      the <code>-Xss</code> JVM option.</li>
+</ul>
+
 </div>
 <div class="footer">
   <span class="right"><a href="@jacoco.home.url@">JaCoCo</a> @qualified.bundle.version@</span>