6890: defineEventProbes should throw exception on malformed probe def…

…initions

Reviewed-by: hirt

agent/src/main/java/org/openjdk/jmc/agent/Agent.java

@@ -102,7 +102,7 @@ public static void agentmain(String agentArguments, Instrumentation instrumentat
 	 *             if the configuration could not be read.
 	 */
 	public static void initializeAgent(InputStream configuration, Instrumentation instrumentation)
-			throws XMLStreamException {
+			throws XMLStreamException, XMLValidationException {
 		TransformRegistry registry = configuration != null ? DefaultTransformRegistry.from(configuration)
 				: DefaultTransformRegistry.empty();
 		instrumentation.addTransformer(new Transformer(registry), true);
@@ -132,7 +132,7 @@ private static void initializeAgent(String agentArguments, Instrumentation instr
 		if (agentArguments == null || agentArguments.trim().length() == 0) {
 			try {
 				initializeAgent((InputStream) null, instrumentation);
-			} catch (XMLStreamException e) {
+			} catch (XMLStreamException | XMLValidationException e) {
 				// noop: null as InputStream causes defaults to be used - the stream will not be used
 			}
 			return;
@@ -141,7 +141,7 @@ private static void initializeAgent(String agentArguments, Instrumentation instr
 		File file = new File(agentArguments);
 		try (InputStream stream = new FileInputStream(file)) {
 			initializeAgent(stream, instrumentation);
-		} catch (XMLStreamException | IOException e) {
+		} catch (XMLStreamException | IOException | XMLValidationException e) {
 			getLogger().log(Level.SEVERE, "Failed to read jfr probe definitions from " + file.getPath(), e); //$NON-NLS-1$
 		}
 	}

agent/src/main/java/org/openjdk/jmc/agent/TransformRegistry.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2021, Oracle and/or its affiliates. All rights reserved.
  *
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
@@ -83,8 +83,10 @@ public interface TransformRegistry {
 	 * @param xmlDescription
 	 *            an XML snippet describing the wanted modifications.
 	 * @return a set of class names associated with modified {@link TransformDescriptor}s.
+	 * @throws XMLValidationException
+	 *             if the supplied XML fails to validate.
 	 */
-	Set<String> modify(String xmlDescription);
+	Set<String> modify(String xmlDescription) throws XMLValidationException;
 
 	/**
 	 * Clears all classes and their corresponding transforms in the registry.

agent/src/main/java/org/openjdk/jmc/agent/XMLValidationException.java

@@ -0,0 +1,60 @@
+/*
+ * Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2021, Red Hat Inc. All rights reserved.
+ * 
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * The contents of this file are subject to the terms of either the Universal Permissive License
+ * v 1.0 as shown at http://oss.oracle.com/licenses/upl
+ *
+ * or the following license:
+ *
+ * Redistribution and use in source and binary forms, with or without modification, are permitted
+ * provided that the following conditions are met:
+ * 
+ * 1. Redistributions of source code must retain the above copyright notice, this list of conditions
+ * and the following disclaimer.
+ * 
+ * 2. Redistributions in binary form must reproduce the above copyright notice, this list of
+ * conditions and the following disclaimer in the documentation and/or other materials provided with
+ * the distribution.
+ * 
+ * 3. Neither the name of the copyright holder nor the names of its contributors may be used to
+ * endorse or promote products derived from this software without specific prior written permission.
+ * 
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND ANY EXPRESS OR
+ * IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND
+ * FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY,
+ * WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY
+ * WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+package org.openjdk.jmc.agent;
+
+/**
+ * Exception to throw when validating the agent probe definitino xml fails
+ */
+public class XMLValidationException extends Exception {
+
+	/**
+	 */
+	private static final long serialVersionUID = -9001765953540356861L;
+
+	/**
+	 * @param message
+	 */
+	public XMLValidationException(String message) {
+		super(message);
+	}
+
+	/**
+	 * @param message
+	 * @param cause
+	 */
+	public XMLValidationException(String message, Throwable cause) {
+		super(message, cause);
+	}
+
+}

agent/src/main/java/org/openjdk/jmc/agent/impl/DefaultTransformRegistry.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2021, Oracle and/or its affiliates. All rights reserved.
  *
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
@@ -68,6 +68,7 @@
 import org.openjdk.jmc.agent.ReturnValue;
 import org.openjdk.jmc.agent.TransformDescriptor;
 import org.openjdk.jmc.agent.TransformRegistry;
+import org.openjdk.jmc.agent.XMLValidationException;
 import org.openjdk.jmc.agent.jfr.JFRTransformDescriptor;
 import org.openjdk.jmc.agent.util.IOToolkit;
 import org.openjdk.jmc.agent.util.TypeUtils;
@@ -121,20 +122,20 @@ public static TransformRegistry empty() {
 		return new DefaultTransformRegistry();
 	}
 
-	public static void validateProbeDefinition(InputStream in) throws XMLStreamException {
+	public static void validateProbeDefinition(InputStream in) throws XMLValidationException {
 		try {
 			Validator validator = PROBE_SCHEMA.newValidator();
 			validator.validate(new StreamSource(in));
 		} catch (IOException | SAXException e) {
-			throw new XMLStreamException(e);
+			throw new XMLValidationException(e.getMessage(), e);
 		}
 	}
 
-	public static void validateProbeDefinition(String configuration) throws XMLStreamException {
+	public static void validateProbeDefinition(String configuration) throws XMLValidationException {
 		validateProbeDefinition(new ByteArrayInputStream(configuration.getBytes()));
 	}
 
-	public static TransformRegistry from(InputStream in) throws XMLStreamException {
+	public static TransformRegistry from(InputStream in) throws XMLStreamException, XMLValidationException {
 		byte[] buf;
 		InputStream configuration;
 		try {
@@ -145,6 +146,8 @@ public static TransformRegistry from(InputStream in) throws XMLStreamException {
 			configuration.reset();
 		} catch (IOException e) {
 			throw new XMLStreamException(e);
+		} catch (XMLValidationException xve) {
+			throw xve;
 		}
 
 		HashMap<String, String> globalDefaults = new HashMap<>();
@@ -484,7 +487,7 @@ public String toString() {
 	}
 
 	@Override
-	public Set<String> modify(String xmlDescription) {
+	public Set<String> modify(String xmlDescription) throws XMLValidationException {
 		try {
 			validateProbeDefinition(xmlDescription);
 
@@ -517,7 +520,7 @@ public Set<String> modify(String xmlDescription) {
 			return modifiedClasses;
 		} catch (XMLStreamException xse) {
 			logger.log(Level.SEVERE, "Failed to create XML Stream Reader", xse);
-			return null;
+			throw new XMLValidationException(xse.getMessage(), xse);
 		}
 	}
 

agent/src/main/java/org/openjdk/jmc/agent/jmx/AgentController.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2021 Oracle and/or its affiliates. All rights reserved.
  * 
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
@@ -66,14 +66,15 @@ public void defineEventProbes(String xmlDescription) throws Exception {
 			registry.setCurrentConfiguration("");
 		} else {
 			Set<String> initialClasses = new HashSet<>(registry.getClassNames());
-			Set<String> modifiedClasses = registry.modify(xmlDescription);
-			if (modifiedClasses == null) {
-				logger.log(Level.SEVERE, "Failed to identify transformations: " + xmlDescription);
-				return;
-			} else {
-				modifiedClasses.addAll(initialClasses);
-				classesToRetransformArray = retransformClasses(modifiedClasses);
+			Set<String> modifiedClasses;
+			try {
+				modifiedClasses = registry.modify(xmlDescription);
+			} catch (Exception e) {
+				logger.severe("Failed to identify transformations: " + xmlDescription);
+				throw e;
 			}
+			modifiedClasses.addAll(initialClasses);
+			classesToRetransformArray = retransformClasses(modifiedClasses);
 		}
 		registry.setRevertInstrumentation(true);
 		instrumentation.retransformClasses(classesToRetransformArray);

agent/src/test/java/org/openjdk/jmc/agent/converters/test/TestConverterTransforms.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2021 Oracle and/or its affiliates. All rights reserved.
  * 
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
@@ -52,6 +52,7 @@
 import org.objectweb.asm.util.TraceClassVisitor;
 import org.openjdk.jmc.agent.TransformRegistry;
 import org.openjdk.jmc.agent.Transformer;
+import org.openjdk.jmc.agent.XMLValidationException;
 import org.openjdk.jmc.agent.impl.DefaultTransformRegistry;
 import org.openjdk.jmc.agent.test.util.TestToolkit;
 
@@ -63,7 +64,8 @@ public static String getTemplate() throws IOException {
 	}
 
 	@Test
-	public void testRunConverterTransforms() throws XMLStreamException, IllegalClassFormatException, IOException {
+	public void testRunConverterTransforms()
+			throws XMLStreamException, IllegalClassFormatException, IOException, XMLValidationException {
 		TransformRegistry registry = DefaultTransformRegistry.from(TestToolkit.getProbesXMLFromTemplate(getTemplate(),
 				"testRunConverterTransforms" + runCount.getAndIncrement())); //$NON-NLS-1$
 
@@ -84,7 +86,8 @@ public void testRunConverterTransforms() throws XMLStreamException, IllegalClass
 		reader.accept(checkAdapter, 0);
 	}
 
-	public static void main(String[] args) throws XMLStreamException, IOException, IllegalClassFormatException {
+	public static void main(String[] args)
+			throws XMLStreamException, IOException, IllegalClassFormatException, XMLValidationException {
 		TransformRegistry registry = DefaultTransformRegistry.from(TestToolkit.getProbesXMLFromTemplate(getTemplate(),
 				"testRunConverterTransforms" + runCount.getAndIncrement())); //$NON-NLS-1$
 

agent/src/test/java/org/openjdk/jmc/agent/test/TestDefaultTransformRegistry.java

@@ -47,6 +47,7 @@
 import org.objectweb.asm.Type;
 import org.openjdk.jmc.agent.TransformDescriptor;
 import org.openjdk.jmc.agent.TransformRegistry;
+import org.openjdk.jmc.agent.XMLValidationException;
 import org.openjdk.jmc.agent.impl.DefaultTransformRegistry;
 import org.openjdk.jmc.agent.test.util.TestToolkit;
 
@@ -64,7 +65,7 @@ public static String getTemplate() throws IOException {
 	}
 
 	@Test
-	public void testHasPendingTransforms() throws XMLStreamException, IOException {
+	public void testHasPendingTransforms() throws XMLStreamException, IOException, XMLValidationException {
 		TransformRegistry registry = DefaultTransformRegistry
 				.from(TestToolkit.getProbesXMLFromTemplate(getTemplate(), "HasPendingTransforms")); //$NON-NLS-1$
 		assertNotNull(registry);
@@ -78,14 +79,14 @@ public void testEmpty() {
 	}
 
 	@Test
-	public void testFrom() throws XMLStreamException, IOException {
+	public void testFrom() throws XMLStreamException, IOException, XMLValidationException {
 		TransformRegistry registry = DefaultTransformRegistry
 				.from(TestToolkit.getProbesXMLFromTemplate(getTemplate(), "From")); //$NON-NLS-1$
 		assertNotNull(registry);
 	}
 
 	@Test
-	public void testGetTransformData() throws XMLStreamException, IOException {
+	public void testGetTransformData() throws XMLStreamException, IOException, XMLValidationException {
 		TransformRegistry registry = DefaultTransformRegistry
 				.from(TestToolkit.getProbesXMLFromTemplate(getTemplate(), "GetTransformData")); //$NON-NLS-1$
 		assertNotNull(registry);
@@ -95,7 +96,7 @@ public void testGetTransformData() throws XMLStreamException, IOException {
 	}
 
 	@Test
-	public void testModify() throws XMLStreamException, IOException {
+	public void testModify() throws XMLValidationException, IOException, XMLStreamException {
 		TransformRegistry registry = DefaultTransformRegistry
 				.from(TestToolkit.getProbesXMLFromTemplate(getTemplate(), "Modify")); //$NON-NLS-1$
 		assertNotNull(registry);
@@ -107,7 +108,7 @@ public void testModify() throws XMLStreamException, IOException {
 	}
 
 	@Test
-	public void testModifyNameCollision() throws XMLStreamException, IOException {
+	public void testModifyNameCollision() throws XMLValidationException, IOException, XMLStreamException {
 		TransformRegistry registry = DefaultTransformRegistry
 				.from(TestToolkit.getProbesXMLFromTemplate(getTemplate(), "Modify")); //$NON-NLS-1$
 		assertNotNull(registry);
@@ -118,19 +119,25 @@ public void testModifyNameCollision() throws XMLStreamException, IOException {
 	}
 
 	@Test
-	public void testModifyInvalidXml() throws XMLStreamException, IOException {
+	public void testModifyInvalidXml() throws XMLStreamException, IOException, XMLValidationException {
 		TransformRegistry registry = DefaultTransformRegistry
 				.from(TestToolkit.getProbesXMLFromTemplate(getTemplate(), "Modify")); //$NON-NLS-1$
 		assertNotNull(registry);
 		final String initialConfiguration = registry.getCurrentConfiguration();
 		final String invalidSnippet = XML_EVENT_DESCRIPTION;
-		Set<String> modifiedClassNames = registry.modify(invalidSnippet);
-		assertNull(modifiedClassNames);
+		boolean exceptionThrown = false;
+		try {
+			Set<String> modifiedClassNames = registry.modify(invalidSnippet);
+		} catch (Exception e) {
+			e.printStackTrace(System.err);
+			exceptionThrown = true;
+		}
+		assertTrue(exceptionThrown);
 		assertEquals(registry.getCurrentConfiguration(), initialConfiguration);
 	}
 
 	@Test
-	public void testClearAllTransformData() throws XMLStreamException, IOException {
+	public void testClearAllTransformData() throws XMLStreamException, IOException, XMLValidationException {
 		TransformRegistry registry = DefaultTransformRegistry.from(TestToolkit
 				.getProbesXMLFromTemplate(getXMLDescription(XML_EVENT_DESCRIPTION), "clearAllTransformData")); //$NON-NLS-1$
 		assertNotNull(registry);

agent/src/test/java/org/openjdk/jmc/agent/test/TestDefineEventProbes.java

@@ -1,6 +1,6 @@
 /*
- * Copyright (c) 2018, 2020, Oracle and/or its affiliates. All rights reserved.
- * Copyright (c) 2019, 2020, Red Hat Inc. All rights reserved.
+ * Copyright (c) 2018, 2021, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2019, 2021, Red Hat Inc. All rights reserved.
  * 
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
@@ -80,6 +80,8 @@ public class TestDefineEventProbes {
 			+ EVENT_PATH + "</path>" + "<stacktrace>true</stacktrace>" + "<class>" + EVENT_CLASS_NAME + "</class>"
 			+ "<method>" + "<name>" + METHOD_NAME + "</name>" + "<descriptor>" + METHOD_DESCRIPTOR + "</descriptor>"
 			+ "</method>" + "<location>WRAP</location>" + "</event>" + "</events>" + "</jfragent>";
+	private static final String MALFORMED_XML = "<this>" + "<is>" + "<not>" + "<a>" + "<valid>" + "<probe>"
+			+ "<definition>" + "</definition>" + "</probe>" + "</valid>" + "</a>" + "</not>" + "</is>" + "</this>";
 
 	@Test
 	public void testDefineEventProbes() throws Exception {
@@ -112,6 +114,18 @@ public void testDefineEventProbes() throws Exception {
 		assertFalse(exceptionThrown);
 	}
 
+	@Test
+	public void testMalformedProbeDefinition() throws Exception {
+		boolean exceptionThrown = false;
+		try {
+			doDefineEventProbes(MALFORMED_XML);
+		} catch (Exception e) {
+			exceptionThrown = true;
+			e.printStackTrace(System.err);
+		}
+		assertTrue(exceptionThrown);
+	}
+
 	private void injectFailingEvent() throws Exception {
 		Method method = new Method(METHOD_NAME, METHOD_DESCRIPTOR);
 		Map<String, String> attributes = new HashMap<>();

agent/src/test/java/org/openjdk/jmc/agent/test/TestJFRTransformer.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2021 Oracle and/or its affiliates. All rights reserved.
  * 
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
@@ -52,14 +52,16 @@
 import org.openjdk.jmc.agent.Agent;
 import org.openjdk.jmc.agent.TransformRegistry;
 import org.openjdk.jmc.agent.Transformer;
+import org.openjdk.jmc.agent.XMLValidationException;
 import org.openjdk.jmc.agent.impl.DefaultTransformRegistry;
 import org.openjdk.jmc.agent.test.util.TestToolkit;
 
 public class TestJFRTransformer {
 	private static AtomicInteger runCount = new AtomicInteger(0);
 
 	@Test
-	public void testRunTransforms() throws XMLStreamException, IllegalClassFormatException, IOException {
+	public void testRunTransforms()
+			throws XMLStreamException, XMLValidationException, IllegalClassFormatException, IOException {
 		TransformRegistry registry = DefaultTransformRegistry.from(TestToolkit.getProbesXMLFromTemplate(
 				TestDefaultTransformRegistry.getTemplate(), "RunTransforms" + runCount.getAndIncrement())); //$NON-NLS-1$