diff --git a/commafeed-server/src/main/java/com/commafeed/backend/feed/parser/FeedParser.java b/commafeed-server/src/main/java/com/commafeed/backend/feed/parser/FeedParser.java index 68b89deb..ac626296 100644 --- a/commafeed-server/src/main/java/com/commafeed/backend/feed/parser/FeedParser.java +++ b/commafeed-server/src/main/java/com/commafeed/backend/feed/parser/FeedParser.java @@ -53,14 +53,14 @@ public class FeedParser { private static final Comparator 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); } diff --git a/commafeed-server/src/main/java/com/commafeed/backend/feed/parser/FeedCleaner.java b/commafeed-server/src/main/java/com/commafeed/backend/feed/parser/XMLCleaner.java similarity index 98% rename from commafeed-server/src/main/java/com/commafeed/backend/feed/parser/FeedCleaner.java rename to commafeed-server/src/main/java/com/commafeed/backend/feed/parser/XMLCleaner.java index 127a388e..358deea9 100644 --- a/commafeed-server/src/main/java/com/commafeed/backend/feed/parser/FeedCleaner.java +++ b/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("]*>", Pattern.CASE_INSENSITIVE); diff --git a/commafeed-server/src/main/java/com/commafeed/backend/opml/OPMLImporter.java b/commafeed-server/src/main/java/com/commafeed/backend/opml/OPMLImporter.java index ac575c5b..8953a9a0 100644 --- a/commafeed-server/src/main/java/com/commafeed/backend/opml/OPMLImporter.java +++ b/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 outlines = feed.getOutlines(); diff --git a/commafeed-server/src/test/java/com/commafeed/backend/feed/parser/FeedCleanerTest.java b/commafeed-server/src/test/java/com/commafeed/backend/feed/parser/XMLCleanerTest.java similarity index 57% rename from commafeed-server/src/test/java/com/commafeed/backend/feed/parser/FeedCleanerTest.java rename to commafeed-server/src/test/java/com/commafeed/backend/feed/parser/XMLCleanerTest.java index d1afafd2..a1cba3b8 100644 --- a/commafeed-server/src/test/java/com/commafeed/backend/feed/parser/FeedCleanerTest.java +++ b/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\tcontent"; - Assertions.assertEquals("content", feedCleaner.removeCharactersBeforeFirstXmlTag(xml)); + Assertions.assertEquals("content", xmlCleaner.removeCharactersBeforeFirstXmlTag(xml)); } @Test void removesTextBeforeXmlTag() { String xml = "some text herecontent"; - Assertions.assertEquals("content", feedCleaner.removeCharactersBeforeFirstXmlTag(xml)); + Assertions.assertEquals("content", xmlCleaner.removeCharactersBeforeFirstXmlTag(xml)); } @Test void returnsUnchangedWhenStartsWithXmlTag() { String xml = "content"; - Assertions.assertEquals("content", feedCleaner.removeCharactersBeforeFirstXmlTag(xml)); + Assertions.assertEquals("content", 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 = "garbagecontent"; - Assertions.assertEquals("content", feedCleaner.removeCharactersBeforeFirstXmlTag(xml)); + Assertions.assertEquals("content", xmlCleaner.removeCharactersBeforeFirstXmlTag(xml)); } } @@ -61,58 +61,58 @@ class FeedCleanerTest { @Test void removesNullCharacter() { String xml = "content\u0000here"; - Assertions.assertEquals("contenthere", feedCleaner.removeInvalidXmlCharacters(xml)); + Assertions.assertEquals("contenthere", xmlCleaner.removeInvalidXmlCharacters(xml)); } @Test void removesInvalidControlCharacters() { String xml = "content\u0001\u0002\u0003here"; - Assertions.assertEquals("contenthere", feedCleaner.removeInvalidXmlCharacters(xml)); + Assertions.assertEquals("contenthere", xmlCleaner.removeInvalidXmlCharacters(xml)); } @Test void preservesValidXmlCharacters() { String xml = "content with\ttab\nand newline"; - Assertions.assertEquals("content with\ttab\nand newline", feedCleaner.removeInvalidXmlCharacters(xml)); + Assertions.assertEquals("content with\ttab\nand newline", xmlCleaner.removeInvalidXmlCharacters(xml)); } @Test void preservesUnicodeCharacters() { String xml = "café résumé 中文 العربية"; - Assertions.assertEquals("café résumé 中文 العربية", feedCleaner.removeInvalidXmlCharacters(xml)); + Assertions.assertEquals("café résumé 中文 العربية", xmlCleaner.removeInvalidXmlCharacters(xml)); } @Test void preservesEmojiCharacters() { String xml = "🎮💪✅"; - Assertions.assertEquals("🎮💪✅", feedCleaner.removeInvalidXmlCharacters(xml)); + Assertions.assertEquals("🎮💪✅", 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 = "T´l´phone ′"; Assertions.assertEquals("T´l´phone ′", - feedCleaner.replaceHtmlEntitiesWithNumericEntities(source)); + xmlCleaner.replaceHtmlEntitiesWithNumericEntities(source)); } @Test void replacesMultipleOccurrencesOfSameEntity() { String source = "   "; - Assertions.assertEquals("   ", feedCleaner.replaceHtmlEntitiesWithNumericEntities(source)); + Assertions.assertEquals("   ", xmlCleaner.replaceHtmlEntitiesWithNumericEntities(source)); } @Test void preservesTextWithoutEntities() { String source = "regular content"; - Assertions.assertEquals("regular content", feedCleaner.replaceHtmlEntitiesWithNumericEntities(source)); + Assertions.assertEquals("regular content", xmlCleaner.replaceHtmlEntitiesWithNumericEntities(source)); } @Test void preservesNumericEntities() { String source = "´′"; - Assertions.assertEquals("´′", feedCleaner.replaceHtmlEntitiesWithNumericEntities(source)); + Assertions.assertEquals("´′", xmlCleaner.replaceHtmlEntitiesWithNumericEntities(source)); } @Test void replacesCommonHtmlEntities() { String source = "&""; - Assertions.assertEquals("&"", feedCleaner.replaceHtmlEntitiesWithNumericEntities(source)); + Assertions.assertEquals("&"", xmlCleaner.replaceHtmlEntitiesWithNumericEntities(source)); } @Test void handlesPartialEntityMatches() { String source = "&lifier"; - String result = feedCleaner.replaceHtmlEntitiesWithNumericEntities(source); + String result = xmlCleaner.replaceHtmlEntitiesWithNumericEntities(source); Assertions.assertTrue(result.startsWith("&") || result.equals("&lifier")); } @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 = "&test"; - Assertions.assertEquals("&test", feedCleaner.replaceHtmlEntitiesWithNumericEntities(source)); + Assertions.assertEquals("&test", xmlCleaner.replaceHtmlEntitiesWithNumericEntities(source)); } @Test void handlesEntityAtEndOfString() { String source = "test&"; - Assertions.assertEquals("test&", feedCleaner.replaceHtmlEntitiesWithNumericEntities(source)); + Assertions.assertEquals("test&", xmlCleaner.replaceHtmlEntitiesWithNumericEntities(source)); } @Test void handlesMixedEntitiesAndText() { String source = "Hello World! Test."; - 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 = ""; - Assertions.assertEquals("", feedCleaner.removeDoctypeDeclarations(source)); + Assertions.assertEquals("", xmlCleaner.removeDoctypeDeclarations(source)); } @Test @@ -208,64 +208,64 @@ class FeedCleanerTest { """; Assertions.assertEquals(""" - """, feedCleaner.removeDoctypeDeclarations(source)); + """, xmlCleaner.removeDoctypeDeclarations(source)); } @Test void removesComplexDoctypeWithSystemId() { String source = ""; - Assertions.assertEquals("", feedCleaner.removeDoctypeDeclarations(source)); + Assertions.assertEquals("", xmlCleaner.removeDoctypeDeclarations(source)); } @Test void removesComplexDoctypeWithPublicId() { String source = ""; - Assertions.assertEquals("", feedCleaner.removeDoctypeDeclarations(source)); + Assertions.assertEquals("", xmlCleaner.removeDoctypeDeclarations(source)); } @Test void removesCaseInsensitiveDoctype() { String source = ""; - Assertions.assertEquals("", feedCleaner.removeDoctypeDeclarations(source)); + Assertions.assertEquals("", xmlCleaner.removeDoctypeDeclarations(source)); } @Test void removesMixedCaseDoctype() { String source = ""; - Assertions.assertEquals("", feedCleaner.removeDoctypeDeclarations(source)); + Assertions.assertEquals("", xmlCleaner.removeDoctypeDeclarations(source)); } @Test void removesMultipleDoctypeDeclarations() { String source = ""; - Assertions.assertEquals("", feedCleaner.removeDoctypeDeclarations(source)); + Assertions.assertEquals("", xmlCleaner.removeDoctypeDeclarations(source)); } @Test void preservesContentWithoutDoctype() { String source = "No doctype here"; - Assertions.assertEquals("No doctype here", feedCleaner.removeDoctypeDeclarations(source)); + Assertions.assertEquals("No doctype here", 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 = ""; - Assertions.assertEquals("", feedCleaner.removeDoctypeDeclarations(source)); + Assertions.assertEquals("", xmlCleaner.removeDoctypeDeclarations(source)); } } } diff --git a/commafeed-server/src/test/java/com/commafeed/backend/opml/OPMLImporterTest.java b/commafeed-server/src/test/java/com/commafeed/backend/opml/OPMLImporterTest.java index 1193bc3f..87101820 100644 --- a/commafeed-server/src/test/java/com/commafeed/backend/opml/OPMLImporterTest.java +++ b/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)