apply xml cleaning to opml files

commafeed-server/src/main/java/com/commafeed/backend/feed/parser/FeedParser.java

@@ -53,14 +53,14 @@ public class FeedParser {
 	private static final Comparator<Entry> ENTRY_COMPARATOR = Comparator.comparing(Entry::published).reversed();
 
 	private final EncodingDetector encodingDetector;
-	private final FeedCleaner feedCleaner;
+	private final XMLCleaner xmlCleaner;
 
-	public FeedParser(EncodingDetector encodingDetector, FeedCleaner feedCleaner) {
+	public FeedParser(EncodingDetector encodingDetector, XMLCleaner xmlCleaner) {
 		this.encodingDetector = encodingDetector;
-		this.feedCleaner = feedCleaner;
+		this.xmlCleaner = xmlCleaner;
 
 		// disable entity expansion limits added in JDK24+ (#1961)
-		// we already strip doctype declarations in FeedCleaner to prevent xxe attacks
+		// we already strip doctype declarations in XMLCleaner to prevent xxe attacks
 		// we also already limit the size of feeds we download in HttpGetter
 		System.setProperty(SystemProperties.JDK_XML_MAX_GENERAL_ENTITY_SIZE_LIMIT, "0");
 		System.setProperty(SystemProperties.JDK_XML_TOTAL_ENTITY_SIZE_LIMIT, "0");
@@ -70,7 +70,7 @@ public class FeedParser {
 		try {
 			Charset encoding = encodingDetector.getEncoding(xml);
 
-			String xmlString = feedCleaner.clean(new String(xml, encoding));
+			String xmlString = xmlCleaner.clean(new String(xml, encoding));
 			if (xmlString == null) {
 				throw new FeedParsingException("Input string is empty for url " + feedUrl);
 			}

commafeed-server/src/main/java/com/commafeed/backend/feed/parser/XMLCleaner.java

@@ -11,7 +11,7 @@ import org.apache.commons.lang3.StringUtils;
 import org.jdom2.Verifier;
 
 @Singleton
-public class FeedCleaner {
+public class XMLCleaner {
 
 	private static final Pattern DOCTYPE_PATTERN = Pattern.compile("<!DOCTYPE[^>]*>", Pattern.CASE_INSENSITIVE);
 

commafeed-server/src/main/java/com/commafeed/backend/opml/OPMLImporter.java

@@ -10,6 +10,7 @@ import org.apache.commons.lang3.StringUtils;
 
 import com.commafeed.backend.dao.FeedCategoryDAO;
 import com.commafeed.backend.feed.FeedUtils;
+import com.commafeed.backend.feed.parser.XMLCleaner;
 import com.commafeed.backend.model.FeedCategory;
 import com.commafeed.backend.model.User;
 import com.commafeed.backend.service.FeedSubscriptionService;
@@ -26,15 +27,16 @@ import lombok.extern.slf4j.Slf4j;
 @Singleton
 public class OPMLImporter {
 
+	private final XMLCleaner xmlCleaner;
 	private final FeedCategoryDAO feedCategoryDAO;
 	private final FeedSubscriptionService feedSubscriptionService;
 
 	public void importOpml(User user, String xml) throws IllegalArgumentException, FeedException {
-		int index = xml.indexOf('<');
-		if (index == -1) {
-			throw new IllegalArgumentException("Invalid OPML: no start tag found");
+		xml = xmlCleaner.clean(xml);
+		if (xml == null) {
+			throw new IllegalArgumentException("Invalid OPML");
 		}
-		xml = xml.substring(index);
+
 		WireFeedInput input = new WireFeedInput();
 		Opml feed = (Opml) input.build(new StringReader(xml));
 		List<Outline> outlines = feed.getOutlines();

commafeed-server/src/test/java/com/commafeed/backend/feed/parser/XMLCleanerTest.java

@@ -4,55 +4,55 @@ import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.Nested;
 import org.junit.jupiter.api.Test;
 
-class FeedCleanerTest {
+class XMLCleanerTest {
 
-	FeedCleaner feedCleaner = new FeedCleaner();
+	XMLCleaner xmlCleaner = new XMLCleaner();
 
 	@Nested
 	class RemoveCharactersBeforeFirstXmlTag {
 		@Test
 		void removesWhitespaceBeforeXmlTag() {
 			String xml = "   \n\t<feed>content</feed>";
-			Assertions.assertEquals("<feed>content</feed>", feedCleaner.removeCharactersBeforeFirstXmlTag(xml));
+			Assertions.assertEquals("<feed>content</feed>", xmlCleaner.removeCharactersBeforeFirstXmlTag(xml));
 		}
 
 		@Test
 		void removesTextBeforeXmlTag() {
 			String xml = "some text here<feed>content</feed>";
-			Assertions.assertEquals("<feed>content</feed>", feedCleaner.removeCharactersBeforeFirstXmlTag(xml));
+			Assertions.assertEquals("<feed>content</feed>", xmlCleaner.removeCharactersBeforeFirstXmlTag(xml));
 		}
 
 		@Test
 		void returnsUnchangedWhenStartsWithXmlTag() {
 			String xml = "<feed>content</feed>";
-			Assertions.assertEquals("<feed>content</feed>", feedCleaner.removeCharactersBeforeFirstXmlTag(xml));
+			Assertions.assertEquals("<feed>content</feed>", xmlCleaner.removeCharactersBeforeFirstXmlTag(xml));
 		}
 
 		@Test
 		void returnsNullWhenNoXmlTagFound() {
 			String xml = "no xml tags here";
-			Assertions.assertNull(feedCleaner.removeCharactersBeforeFirstXmlTag(xml));
+			Assertions.assertNull(xmlCleaner.removeCharactersBeforeFirstXmlTag(xml));
 		}
 
 		@Test
 		void returnsNullWhenInputIsNull() {
-			Assertions.assertNull(feedCleaner.removeCharactersBeforeFirstXmlTag(null));
+			Assertions.assertNull(xmlCleaner.removeCharactersBeforeFirstXmlTag(null));
 		}
 
 		@Test
 		void returnsNullWhenInputIsEmpty() {
-			Assertions.assertNull(feedCleaner.removeCharactersBeforeFirstXmlTag(""));
+			Assertions.assertNull(xmlCleaner.removeCharactersBeforeFirstXmlTag(""));
 		}
 
 		@Test
 		void returnsNullWhenInputIsBlank() {
-			Assertions.assertNull(feedCleaner.removeCharactersBeforeFirstXmlTag("   \n\t  "));
+			Assertions.assertNull(xmlCleaner.removeCharactersBeforeFirstXmlTag("   \n\t  "));
 		}
 
 		@Test
 		void preservesMultipleXmlTags() {
 			String xml = "garbage<feed><item>content</item></feed>";
-			Assertions.assertEquals("<feed><item>content</item></feed>", feedCleaner.removeCharactersBeforeFirstXmlTag(xml));
+			Assertions.assertEquals("<feed><item>content</item></feed>", xmlCleaner.removeCharactersBeforeFirstXmlTag(xml));
 		}
 	}
 
@@ -61,58 +61,58 @@ class FeedCleanerTest {
 		@Test
 		void removesNullCharacter() {
 			String xml = "<feed>content\u0000here</feed>";
-			Assertions.assertEquals("<feed>contenthere</feed>", feedCleaner.removeInvalidXmlCharacters(xml));
+			Assertions.assertEquals("<feed>contenthere</feed>", xmlCleaner.removeInvalidXmlCharacters(xml));
 		}
 
 		@Test
 		void removesInvalidControlCharacters() {
 			String xml = "<feed>content\u0001\u0002\u0003here</feed>";
-			Assertions.assertEquals("<feed>contenthere</feed>", feedCleaner.removeInvalidXmlCharacters(xml));
+			Assertions.assertEquals("<feed>contenthere</feed>", xmlCleaner.removeInvalidXmlCharacters(xml));
 		}
 
 		@Test
 		void preservesValidXmlCharacters() {
 			String xml = "<feed>content with\ttab\nand newline</feed>";
-			Assertions.assertEquals("<feed>content with\ttab\nand newline</feed>", feedCleaner.removeInvalidXmlCharacters(xml));
+			Assertions.assertEquals("<feed>content with\ttab\nand newline</feed>", xmlCleaner.removeInvalidXmlCharacters(xml));
 		}
 
 		@Test
 		void preservesUnicodeCharacters() {
 			String xml = "<feed>café résumé 中文 العربية</feed>";
-			Assertions.assertEquals("<feed>café résumé 中文 العربية</feed>", feedCleaner.removeInvalidXmlCharacters(xml));
+			Assertions.assertEquals("<feed>café résumé 中文 العربية</feed>", xmlCleaner.removeInvalidXmlCharacters(xml));
 		}
 
 		@Test
 		void preservesEmojiCharacters() {
 			String xml = "<feed>🎮💪✅</feed>";
-			Assertions.assertEquals("<feed>🎮💪✅</feed>", feedCleaner.removeInvalidXmlCharacters(xml));
+			Assertions.assertEquals("<feed>🎮💪✅</feed>", xmlCleaner.removeInvalidXmlCharacters(xml));
 		}
 
 		@Test
 		void removesMultipleInvalidCharacters() {
 			String xml = "test\u0000test\u0001test\u0002test";
-			Assertions.assertEquals("testtesttesttest", feedCleaner.removeInvalidXmlCharacters(xml));
+			Assertions.assertEquals("testtesttesttest", xmlCleaner.removeInvalidXmlCharacters(xml));
 		}
 
 		@Test
 		void returnsNullWhenInputIsNull() {
-			Assertions.assertNull(feedCleaner.removeInvalidXmlCharacters(null));
+			Assertions.assertNull(xmlCleaner.removeInvalidXmlCharacters(null));
 		}
 
 		@Test
 		void returnsNullWhenInputIsEmpty() {
-			Assertions.assertNull(feedCleaner.removeInvalidXmlCharacters(""));
+			Assertions.assertNull(xmlCleaner.removeInvalidXmlCharacters(""));
 		}
 
 		@Test
 		void returnsNullWhenInputIsBlank() {
-			Assertions.assertNull(feedCleaner.removeInvalidXmlCharacters("   "));
+			Assertions.assertNull(xmlCleaner.removeInvalidXmlCharacters("   "));
 		}
 
 		@Test
 		void handlesStringWithOnlyInvalidCharacters() {
 			String xml = "\u0000\u0001\u0002";
-			Assertions.assertEquals("", feedCleaner.removeInvalidXmlCharacters(xml));
+			Assertions.assertEquals("", xmlCleaner.removeInvalidXmlCharacters(xml));
 		}
 	}
 
@@ -122,71 +122,71 @@ class FeedCleanerTest {
 		void testReplaceHtmlEntitiesWithNumericEntities() {
 			String source = "<source>T&acute;l&acute;phone &prime;</source>";
 			Assertions.assertEquals("<source>T&#180;l&#180;phone &#8242;</source>",
-					feedCleaner.replaceHtmlEntitiesWithNumericEntities(source));
+					xmlCleaner.replaceHtmlEntitiesWithNumericEntities(source));
 		}
 
 		@Test
 		void replacesMultipleOccurrencesOfSameEntity() {
 			String source = "&nbsp;&nbsp;&nbsp;";
-			Assertions.assertEquals("&#160;&#160;&#160;", feedCleaner.replaceHtmlEntitiesWithNumericEntities(source));
+			Assertions.assertEquals("&#160;&#160;&#160;", xmlCleaner.replaceHtmlEntitiesWithNumericEntities(source));
 		}
 
 		@Test
 		void preservesTextWithoutEntities() {
 			String source = "<feed>regular content</feed>";
-			Assertions.assertEquals("<feed>regular content</feed>", feedCleaner.replaceHtmlEntitiesWithNumericEntities(source));
+			Assertions.assertEquals("<feed>regular content</feed>", xmlCleaner.replaceHtmlEntitiesWithNumericEntities(source));
 		}
 
 		@Test
 		void preservesNumericEntities() {
 			String source = "&#180;&#8242;";
-			Assertions.assertEquals("&#180;&#8242;", feedCleaner.replaceHtmlEntitiesWithNumericEntities(source));
+			Assertions.assertEquals("&#180;&#8242;", xmlCleaner.replaceHtmlEntitiesWithNumericEntities(source));
 		}
 
 		@Test
 		void replacesCommonHtmlEntities() {
 			String source = "&amp;&quot;";
-			Assertions.assertEquals("&#38;&#34;", feedCleaner.replaceHtmlEntitiesWithNumericEntities(source));
+			Assertions.assertEquals("&#38;&#34;", xmlCleaner.replaceHtmlEntitiesWithNumericEntities(source));
 		}
 
 		@Test
 		void handlesPartialEntityMatches() {
 			String source = "&amplifier";
-			String result = feedCleaner.replaceHtmlEntitiesWithNumericEntities(source);
+			String result = xmlCleaner.replaceHtmlEntitiesWithNumericEntities(source);
 			Assertions.assertTrue(result.startsWith("&#38;") || result.equals("&amplifier"));
 		}
 
 		@Test
 		void returnsNullWhenInputIsNull() {
-			Assertions.assertNull(feedCleaner.replaceHtmlEntitiesWithNumericEntities(null));
+			Assertions.assertNull(xmlCleaner.replaceHtmlEntitiesWithNumericEntities(null));
 		}
 
 		@Test
 		void returnsNullWhenInputIsEmpty() {
-			Assertions.assertNull(feedCleaner.replaceHtmlEntitiesWithNumericEntities(""));
+			Assertions.assertNull(xmlCleaner.replaceHtmlEntitiesWithNumericEntities(""));
 		}
 
 		@Test
 		void returnsNullWhenInputIsBlank() {
-			Assertions.assertNull(feedCleaner.replaceHtmlEntitiesWithNumericEntities("   "));
+			Assertions.assertNull(xmlCleaner.replaceHtmlEntitiesWithNumericEntities("   "));
 		}
 
 		@Test
 		void handlesEntityAtStartOfString() {
 			String source = "&amp;test";
-			Assertions.assertEquals("&#38;test", feedCleaner.replaceHtmlEntitiesWithNumericEntities(source));
+			Assertions.assertEquals("&#38;test", xmlCleaner.replaceHtmlEntitiesWithNumericEntities(source));
 		}
 
 		@Test
 		void handlesEntityAtEndOfString() {
 			String source = "test&amp;";
-			Assertions.assertEquals("test&#38;", feedCleaner.replaceHtmlEntitiesWithNumericEntities(source));
+			Assertions.assertEquals("test&#38;", xmlCleaner.replaceHtmlEntitiesWithNumericEntities(source));
 		}
 
 		@Test
 		void handlesMixedEntitiesAndText() {
 			String source = "Hello&nbsp;World&excl;&nbsp;Test&period;";
-			String result = feedCleaner.replaceHtmlEntitiesWithNumericEntities(source);
+			String result = xmlCleaner.replaceHtmlEntitiesWithNumericEntities(source);
 			Assertions.assertTrue(result.contains("&#"));
 		}
 	}
@@ -196,7 +196,7 @@ class FeedCleanerTest {
 		@Test
 		void testRemoveDoctype() {
 			String source = "<!DOCTYPE html><html><head></head><body></body></html>";
-			Assertions.assertEquals("<html><head></head><body></body></html>", feedCleaner.removeDoctypeDeclarations(source));
+			Assertions.assertEquals("<html><head></head><body></body></html>", xmlCleaner.removeDoctypeDeclarations(source));
 		}
 
 		@Test
@@ -208,64 +208,64 @@ class FeedCleanerTest {
 					<html><head></head><body></body></html>""";
 			Assertions.assertEquals("""
 
-					<html><head></head><body></body></html>""", feedCleaner.removeDoctypeDeclarations(source));
+					<html><head></head><body></body></html>""", xmlCleaner.removeDoctypeDeclarations(source));
 		}
 
 		@Test
 		void removesComplexDoctypeWithSystemId() {
 			String source = "<!DOCTYPE html SYSTEM \"about:legacy-compat\"><html><body></body></html>";
-			Assertions.assertEquals("<html><body></body></html>", feedCleaner.removeDoctypeDeclarations(source));
+			Assertions.assertEquals("<html><body></body></html>", xmlCleaner.removeDoctypeDeclarations(source));
 		}
 
 		@Test
 		void removesComplexDoctypeWithPublicId() {
 			String source = "<!DOCTYPE html PUBLIC \"-//W3C//DTD XHTML 1.0 Strict//EN\" \"http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd\"><html></html>";
-			Assertions.assertEquals("<html></html>", feedCleaner.removeDoctypeDeclarations(source));
+			Assertions.assertEquals("<html></html>", xmlCleaner.removeDoctypeDeclarations(source));
 		}
 
 		@Test
 		void removesCaseInsensitiveDoctype() {
 			String source = "<!doctype html><html></html>";
-			Assertions.assertEquals("<html></html>", feedCleaner.removeDoctypeDeclarations(source));
+			Assertions.assertEquals("<html></html>", xmlCleaner.removeDoctypeDeclarations(source));
 		}
 
 		@Test
 		void removesMixedCaseDoctype() {
 			String source = "<!DoCtYpE html><html></html>";
-			Assertions.assertEquals("<html></html>", feedCleaner.removeDoctypeDeclarations(source));
+			Assertions.assertEquals("<html></html>", xmlCleaner.removeDoctypeDeclarations(source));
 		}
 
 		@Test
 		void removesMultipleDoctypeDeclarations() {
 			String source = "<!DOCTYPE html><!DOCTYPE html><html></html>";
-			Assertions.assertEquals("<html></html>", feedCleaner.removeDoctypeDeclarations(source));
+			Assertions.assertEquals("<html></html>", xmlCleaner.removeDoctypeDeclarations(source));
 		}
 
 		@Test
 		void preservesContentWithoutDoctype() {
 			String source = "<html><body>No doctype here</body></html>";
-			Assertions.assertEquals("<html><body>No doctype here</body></html>", feedCleaner.removeDoctypeDeclarations(source));
+			Assertions.assertEquals("<html><body>No doctype here</body></html>", xmlCleaner.removeDoctypeDeclarations(source));
 		}
 
 		@Test
 		void returnsNullWhenInputIsNull() {
-			Assertions.assertNull(feedCleaner.removeDoctypeDeclarations(null));
+			Assertions.assertNull(xmlCleaner.removeDoctypeDeclarations(null));
 		}
 
 		@Test
 		void returnsNullWhenInputIsEmpty() {
-			Assertions.assertNull(feedCleaner.removeDoctypeDeclarations(""));
+			Assertions.assertNull(xmlCleaner.removeDoctypeDeclarations(""));
 		}
 
 		@Test
 		void returnsNullWhenInputIsBlank() {
-			Assertions.assertNull(feedCleaner.removeDoctypeDeclarations("   "));
+			Assertions.assertNull(xmlCleaner.removeDoctypeDeclarations("   "));
 		}
 
 		@Test
 		void handlesDoctypeWithExtraWhitespace() {
 			String source = "<!DOCTYPE   html   ><html></html>";
-			Assertions.assertEquals("<html></html>", feedCleaner.removeDoctypeDeclarations(source));
+			Assertions.assertEquals("<html></html>", xmlCleaner.removeDoctypeDeclarations(source));
 		}
 	}
 }

commafeed-server/src/test/java/com/commafeed/backend/opml/OPMLImporterTest.java

@@ -8,6 +8,7 @@ import org.junit.jupiter.api.Test;
 import org.mockito.Mockito;
 
 import com.commafeed.backend.dao.FeedCategoryDAO;
+import com.commafeed.backend.feed.parser.XMLCleaner;
 import com.commafeed.backend.model.FeedCategory;
 import com.commafeed.backend.model.User;
 import com.commafeed.backend.service.FeedSubscriptionService;
@@ -36,13 +37,15 @@ class OPMLImporterTest {
 	}
 
 	private void testOpmlVersion(String fileName) throws IOException, IllegalArgumentException, FeedException {
+		XMLCleaner xmlCleaner = Mockito.mock(XMLCleaner.class);
 		FeedCategoryDAO feedCategoryDAO = Mockito.mock(FeedCategoryDAO.class);
 		FeedSubscriptionService feedSubscriptionService = Mockito.mock(FeedSubscriptionService.class);
 		User user = Mockito.mock(User.class);
 
+		Mockito.when(xmlCleaner.clean(Mockito.anyString())).thenAnswer(invocation -> invocation.getArgument(0));
 		String xml = IOUtils.toString(getClass().getResourceAsStream(fileName), StandardCharsets.UTF_8);
 
-		OPMLImporter importer = new OPMLImporter(feedCategoryDAO, feedSubscriptionService);
+		OPMLImporter importer = new OPMLImporter(xmlCleaner, feedCategoryDAO, feedSubscriptionService);
 		importer.importOpml(user, xml);
 
 		Mockito.verify(feedSubscriptionService)