Default missing weight to 1 and harden XML parser against XXE

The DTD declares weight="1" as the default for VALUE and RULE, but
the loader used a non-validating parser, so a missing attribute came
through as "" and crashed the entire load with NumberFormatException.
Read it through a parseWeight helper that treats blank as 1, matching
the DTD's stated default.

Also disable external general/parameter entities, XInclude, and
entity-reference expansion on the DocumentBuilderFactory. External
DTD loading is left on so generator.dtd still resolves through the
EntityResolver. The data files are local and trusted, but XXE
hardening is cheap and stops a malformed file from silently inlining
filesystem contents.

Tests cover both: a VALUE without a weight attribute now loads
without throwing, and an XXE payload referencing a local file does
not leak the file's contents into the parsed data.

Author: Vest

code/src/java/pcgen/core/namegen/NameGenDataLoader.java

@@ -103,6 +103,14 @@ private static DocumentBuilder newDocumentBuilder() throws IOException
 		try
 		{
 			DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
+			// XXE hardening: data files are local but the parser shouldn't
+			// fetch external entities or evaluate parameter entities. We
+			// keep external-DTD loading on so generator.dtd still resolves
+			// through the EntityResolver.
+			factory.setFeature("http://xml.org/sax/features/external-general-entities", false);
+			factory.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
+			factory.setXIncludeAware(false);
+			factory.setExpandEntityReferences(false);
 			return factory.newDocumentBuilder();
 		}
 		catch (ParserConfigurationException e)
@@ -129,7 +137,7 @@ private static String loadList(Element list, VariableHashMap allVars)
 		for (Element child : childElements(list, "VALUE"))
 		{
 			WeightedDataValue dv = new WeightedDataValue(directText(child),
-					Integer.parseInt(child.getAttribute("weight")));
+					parseWeight(child));
 			childElements(child, "SUBVALUE").forEach(sub ->
 					dv.addSubValue(sub.getAttribute("type"), directText(sub)));
 			dataList.add(dv);
@@ -140,7 +148,7 @@ private static String loadList(Element list, VariableHashMap allVars)
 
 	private static String loadRule(Element rule, String id, VariableHashMap allVars)
 	{
-		Rule dataRule = new Rule(allVars, id, id, Integer.parseInt(rule.getAttribute("weight")));
+		Rule dataRule = new Rule(allVars, id, id, parseWeight(rule));
 		for (Element child : childElements(rule))
 		{
 			switch (child.getTagName())
@@ -210,6 +218,22 @@ private static List<Element> childElements(Element parent)
 				.toList();
 	}
 
+	/**
+	 * Reads the {@code weight} attribute and defaults to 1 when absent or
+	 * blank, matching the DTD's {@code weight CDATA "1"} default. The
+	 * project uses a non-validating parser, so DTD-defaulted attributes
+	 * arrive here as {@code ""} rather than {@code "1"}.
+	 */
+	private static int parseWeight(Element element)
+	{
+		String raw = element.getAttribute("weight");
+		if (raw.isBlank())
+		{
+			return 1;
+		}
+		return Integer.parseInt(raw.trim());
+	}
+
 	private static List<Element> childElements(Element parent, String tagName)
 	{
 		NodeList nodes = parent.getChildNodes();

code/src/utest/pcgen/core/namegen/NameGenDataLoaderTest.java

@@ -83,4 +83,57 @@ public void emptyDirectoryYieldsEmptyData(@TempDir Path tempDir) throws IOExcept
 		NameGenData data = NameGenDataLoader.load(tempDir.toFile());
 		assertTrue(data.categories().isEmpty());
 	}
+
+	@Test
+	public void valueWithoutWeightDefaultsToOne(@TempDir Path tempDir) throws IOException
+	{
+		// The DTD declares weight="1" as a default, but we use a
+		// non-validating parser, so a missing weight arrives as "" — the
+		// loader must treat that as 1 instead of throwing.
+		Files.copy(new File(DATA_DIR, "generator.dtd").toPath(),
+				tempDir.resolve("generator.dtd"));
+		Files.writeString(tempDir.resolve("noweight.xml"),
+				"<?xml version=\"1.0\"?><!DOCTYPE GENERATOR SYSTEM \"generator.dtd\">"
+						+ "<GENERATOR>"
+						+ "<LIST title=\"L\" id=\"L\"><VALUE>Donn</VALUE></LIST>"
+						+ "</GENERATOR>");
+		NameGenData data = NameGenDataLoader.load(tempDir.toFile());
+		assertNotNull(data);
+	}
+
+	@Test
+	public void rejectsExternalEntityExpansion(@TempDir Path tempDir) throws IOException
+	{
+		// Classic XXE payload: declare an external entity that points to
+		// a local secret file, then reference it. With external general
+		// entities disabled, the parser must NOT inline the file's
+		// contents into the resulting data — it should either refuse the
+		// document or leave the reference unresolved.
+		Files.copy(new File(DATA_DIR, "generator.dtd").toPath(),
+				tempDir.resolve("generator.dtd"));
+		Path secret = tempDir.resolve("secret.txt");
+		String marker = "TOP-SECRET-CANARY-12345";
+		Files.writeString(secret, marker);
+		String secretUri = secret.toUri().toString();
+		Files.writeString(tempDir.resolve("xxe.xml"),
+				"<?xml version=\"1.0\"?>"
+						+ "<!DOCTYPE GENERATOR ["
+						+ "<!ENTITY leak SYSTEM \"" + secretUri + "\">"
+						+ "]>"
+						+ "<GENERATOR>"
+						+ "<LIST title=\"L\" id=\"L\"><VALUE>prefix-&leak;-suffix</VALUE></LIST>"
+						+ "</GENERATOR>");
+		try
+		{
+			NameGenData data = NameGenDataLoader.load(tempDir.toFile());
+			// If the loader didn't throw, the parsed data must not contain
+			// the secret marker — otherwise XXE expanded successfully.
+			boolean leaked = data.allVars().toString().contains(marker);
+			assertFalse(leaked, "external entity was expanded into the parsed data");
+		}
+		catch (IOException ignored)
+		{
+			// Refusing the document is also an acceptable outcome.
+		}
+	}
 }