Repository: crawler-commons/crawler-commons (java)

Issue:
XMLIndexHandler needs to accumulate the lastmod date string before parsing
Currently the code in `XMLIndexHandler.characters()` just tries to parse whatever it's passed as a date. But this handler can (and will) be passed text incrementally, so it needs to be accumulated, and in the `endElement()` method it can then (a) trim the string, and (b) try to parse the date.

You can see this behavior if you use a `<![CDATA[xxx]]>` format for the `<lastmod>` data, where `characters()` is called with some spaces and a newline, before getting called with the actual string.

Pull request description:
Buffer characters when characters(...) is called, fixes #225 and #226
Buffer characters when characters(...) is called, instead of immediately processing the current character chunk:
- fixes errors (#225 and #226) when character chunks are interrupted by CDATA sections or character entities
- DelegatorHandler provides a character buffer, so that derived classes can append characters to it and finally get the buffered content
- includes code cleanup in all handler classes:
  - add `@Override` annotations
  - remove stubb method implementations


Patch:
diff --git a/CHANGES.txt b/CHANGES.txt
index e9a66f9..84eaea0 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,6 +1,8 @@
 Crawler-Commons Change Log
 
 Current Development 0.11-SNAPSHOT (yyyy-mm-dd)
+- [Sitemaps] XMLHandler needs to append text in characters() vs. immediately processing (kkrugler, sebastian-nagel) #226
+- [Sitemaps] XMLIndexHandler needs to accumulate the lastmod date string before parsing (kkrugler, sebastian-nagel) #225
 - EffectiveTldFinder throws IllegalArgumentException on IDN domain names containing prohibited characters (sebastian-nagel) #231
 - [Sitemaps] Trim Unicode whitespace around URLs (sebastian-nagel, kkrugler) #224
 - [Sitemaps] Sitemap index: stop URL at closing </loc> (sebastian-nagel, kkrugler) #213
diff --git a/src/main/java/crawlercommons/sitemaps/sax/AtomHandler.java b/src/main/java/crawlercommons/sitemaps/sax/AtomHandler.java
index d9e52c1..ddaaab7 100644
--- a/src/main/java/crawlercommons/sitemaps/sax/AtomHandler.java
+++ b/src/main/java/crawlercommons/sitemaps/sax/AtomHandler.java
@@ -77,6 +77,7 @@ class AtomHandler extends DelegatorHandler {
         sitemap.setType(SitemapType.ATOM);
     }
 
+    @Override
     public void startElement(String uri, String localName, String qName, Attributes attributes) throws SAXException {
         if ("entry".equals(localName)) {
             loc = null;
@@ -107,22 +108,25 @@ class AtomHandler extends DelegatorHandler {
         }
     }
 
+    @Override
     public void endElement(String uri, String localName, String qName) throws SAXException {
-        if ("entry".equals(currentElement())) {
+        if ("entry".equals(localName)) {
             maybeAddSiteMapUrl();
-        } else if ("feed".equals(currentElement())) {
+        } else if ("feed".equals(localName)) {
             sitemap.setProcessed(true);
+        } else if ("updated".equals(localName)) {
+            lastMod = getAndResetCharacterBuffer();
         }
     }
 
+    @Override
     public void characters(char[] ch, int start, int length) throws SAXException {
-        String localName = super.currentElement();
-        String value = String.valueOf(ch, start, length);
-        if ("updated".equals(localName)) {
-            lastMod = value;
+        if ("updated".equals(currentElement())) {
+            appendCharacterBuffer(ch, start, length);
         }
     }
 
+    @Override
     public AbstractSiteMap getSiteMap() {
         return sitemap;
     }
@@ -142,10 +146,12 @@ class AtomHandler extends DelegatorHandler {
         lastMod = null;
     }
 
+    @Override
     public void error(SAXParseException e) throws SAXException {
         maybeAddSiteMapUrl();
     }
 
+    @Override
     public void fatalError(SAXParseException e) throws SAXException {
         maybeAddSiteMapUrl();
     }
diff --git a/src/main/java/crawlercommons/sitemaps/sax/DelegatorHandler.java b/src/main/java/crawlercommons/sitemaps/sax/DelegatorHandler.java
index 71bdb1d..fb454e6 100644
--- a/src/main/java/crawlercommons/sitemaps/sax/DelegatorHandler.java
+++ b/src/main/java/crawlercommons/sitemaps/sax/DelegatorHandler.java
@@ -48,6 +48,7 @@ public class DelegatorHandler extends DefaultHandler {
     private UnknownFormatException exception;
     private Set<String> acceptedNamespaces;
     protected Map<String, Extension> extensionNamespaces;
+    private StringBuilder characterBuffer = new StringBuilder();
 
     protected DelegatorHandler(LinkedList<String> elementStack, boolean strict) {
         this.elementStack = elementStack;
@@ -103,6 +104,7 @@ public class DelegatorHandler extends DefaultHandler {
         return exception;
     }
 
+    @Override
     public void startElement(String uri, String localName, String qName, Attributes attributes) throws SAXException {
         if (elementStack.isEmpty() || delegate == null) {
             startRootElement(uri, localName, qName, attributes);
@@ -170,6 +172,7 @@ public class DelegatorHandler extends DefaultHandler {
         delegate.setExtensionNamespaces(extensionNamespaces);
     }
 
+    @Override
     public void endElement(String uri, String localName, String qName) throws SAXException {
         if (delegate != null) {
             delegate.endElement(uri, localName, qName);
@@ -177,12 +180,33 @@ public class DelegatorHandler extends DefaultHandler {
         elementStack.pop();
     }
 
+    @Override
     public void characters(char ch[], int start, int length) throws SAXException {
         if (delegate != null) {
             delegate.characters(ch, start, length);
         }
     }
 
+    protected void appendCharacterBuffer(char ch[], int start, int length) {
+        for (int i = start; i < start + length; i++) {
+            characterBuffer.append(ch[i]);
+        }
+    }
+
+    protected void appendCharacterBuffer(String str) {
+        characterBuffer.append(str);
+    }
+
+    protected String getAndResetCharacterBuffer() {
+        String value = characterBuffer.toString();
+        resetCharacterBuffer();
+        return value;
+    }
+
+    protected void resetCharacterBuffer() {
+        characterBuffer = new StringBuilder();
+    }
+
     protected String currentElement() {
         return elementStack.peek();
     }
@@ -197,12 +221,14 @@ public class DelegatorHandler extends DefaultHandler {
         return delegate.getSiteMap();
     }
 
+    @Override
     public void error(SAXParseException e) throws SAXException {
         if (delegate != null) {
             delegate.error(e);
         }
     }
 
+    @Override
     public void fatalError(SAXParseException e) throws SAXException {
         if (delegate != null) {
             delegate.fatalError(e);
diff --git a/src/main/java/crawlercommons/sitemaps/sax/RSSHandler.java b/src/main/java/crawlercommons/sitemaps/sax/RSSHandler.java
index cc16934..9ca768f 100644
--- a/src/main/java/crawlercommons/sitemaps/sax/RSSHandler.java
+++ b/src/main/java/crawlercommons/sitemaps/sax/RSSHandler.java
@@ -79,7 +79,6 @@ import crawlercommons.sitemaps.AbstractSiteMap.SitemapType;
 class RSSHandler extends DelegatorHandler {
 
     private SiteMap sitemap;
-    private StringBuilder loc;
     private URL locURL;
     private ZonedDateTime lastMod;
     boolean valid;
@@ -88,7 +87,6 @@ class RSSHandler extends DelegatorHandler {
         super(elementStack, strict);
         sitemap = new SiteMap(url);
         sitemap.setType(SitemapType.RSS);
-        loc = new StringBuilder();
     }
 
     /*
@@ -109,47 +107,53 @@ class RSSHandler extends DelegatorHandler {
      */
     @Override
     public void endElement(String uri, String localName, String qName) throws SAXException {
-        if ("link".equals(currentElement()) || "guid".equals(currentElement())) {
+        if ("link".equals(localName)) {
             setLocURL();
-        } else if ("item".equals(currentElement())) {
+        } else if ("guid".equals(localName)) {
+            // accept as link if
+            // - a valid absolute URL (not a URN, UUID or similar)
+            // - and no <link> found yet
+            if (locURL == null) {
+                setLocURL();
+            }
+            resetCharacterBuffer();
+        } else if ("item".equals(localName)) {
             maybeAddSiteMapUrl();
-        } else if ("rss".equals(currentElement())) {
+        } else if ("rss".equals(localName)) {
             sitemap.setProcessed(true);
+        } else if ("pubDate".equals(localName)) {
+            String value = getAndResetCharacterBuffer();
+            lastMod = AbstractSiteMap.parseRSSTimestamp(value);
+            if (lastMod != null && "channel".equals(super.currentElementParent())) {
+                sitemap.setLastModified(lastMod);
+            }
         }
     }
 
     /*
      * (non-Javadoc)
      * 
-     * @see crawlercommons.sitemaps.AbstractSiteMapSAXHandler#characters(char[],
-     * int, int)
+     * @see crawlercommons.sitemaps.sax.DelegatorHandler#characters(char[], int,
+     * int)
      */
     @Override
     public void characters(char[] ch, int start, int length) throws SAXException {
         String localName = super.currentElement();
-        String value = String.valueOf(ch, start, length);
-        if ("pubDate".equals(localName)) {
-            lastMod = AbstractSiteMap.parseRSSTimestamp(value);
-            if (lastMod != null && "channel".equals(super.currentElementParent())) {
-                sitemap.setLastModified(lastMod);
-            }
-        } else if ("link".equals(localName)) {
-            loc.append(value);
-        } else if ("guid".equals(localName)) {
-            // accept as link if
-            // - a valid absolute URL (not a URN, UUID or similar)
-            // - and no <link> found yet
-            if (locURL == null) {
-                loc.append(value);
-            }
+        if ("pubDate".equals(localName) || "link".equals(localName) || "guid".equals(localName)) {
+            appendCharacterBuffer(ch, start, length);
         }
     }
 
+    @Override
     public AbstractSiteMap getSiteMap() {
         return sitemap;
     }
 
     private void setLocURL() {
+        String loc = getAndResetCharacterBuffer();
+        if (loc == null) {
+            return;
+        }
         String value = stripAllBlank(loc);
         if (value.isEmpty()) {
             return;
@@ -160,8 +164,6 @@ class RSSHandler extends DelegatorHandler {
         } catch (MalformedURLException e) {
             LOG.debug("Bad url: [{}]", value);
             LOG.trace("Can't create an entry with a bad URL", e);
-        } finally {
-            loc = new StringBuilder();
         }
     }
 
@@ -178,10 +180,12 @@ class RSSHandler extends DelegatorHandler {
         lastMod = null;
     }
 
+    @Override
     public void error(SAXParseException e) throws SAXException {
         maybeAddSiteMapUrl();
     }
 
+    @Override
     public void fatalError(SAXParseException e) throws SAXException {
         maybeAddSiteMapUrl();
     }
diff --git a/src/main/java/crawlercommons/sitemaps/sax/XMLHandler.java b/src/main/java/crawlercommons/sitemaps/sax/XMLHandler.java
index a161f0d..70e3228 100644
--- a/src/main/java/crawlercommons/sitemaps/sax/XMLHandler.java
+++ b/src/main/java/crawlercommons/sitemaps/sax/XMLHandler.java
@@ -63,7 +63,7 @@ import crawlercommons.sitemaps.sax.extension.ExtensionHandler;
 class XMLHandler extends DelegatorHandler {
 
     private SiteMap sitemap;
-    private StringBuilder loc;
+    private String loc;
     private String lastMod;
     private String changeFreq;
     private String priority;
@@ -76,9 +76,9 @@ class XMLHandler extends DelegatorHandler {
         super(elementStack, strict);
         sitemap = new SiteMap(url);
         sitemap.setType(SitemapType.XML);
-        loc = new StringBuilder();
     }
 
+    @Override
     public void startElement(String uri, String localName, String qName, Attributes attributes) throws SAXException {
         currentElementNamespace = uri;
         if (isExtensionNamespace(uri)) {
@@ -93,12 +93,15 @@ class XMLHandler extends DelegatorHandler {
         currentElementNamespaceIsValid = true;
 
         // flush any unclosed or missing URL element
-        if (loc.length() > 0 && ("loc".equals(localName) || "url".equals(localName))) {
-            if (!isAllBlank(loc)) {
+        if ("loc".equals(localName) || "url".equals(localName)) {
+            if (loc == null) {
+                loc = getAndResetCharacterBuffer();
+            }
+            if (loc != null && !isAllBlank(loc)) {
                 maybeAddSiteMapUrl();
                 return;
             }
-            loc = new StringBuilder();
+            loc = null;
             if ("url".equals(localName)) {
                 // reset also attributes
                 lastMod = null;
@@ -106,8 +109,10 @@ class XMLHandler extends DelegatorHandler {
                 priority = null;
             }
         }
+        resetCharacterBuffer();
     }
 
+    @Override
     public void endElement(String uri, String localName, String qName) throws SAXException {
         if (isExtensionNamespace(uri)) {
             ExtensionHandler eh = getExtensionHandler(uri);
@@ -116,13 +121,24 @@ class XMLHandler extends DelegatorHandler {
         } else if (isStrictNamespace() && !isAcceptedNamespace(uri)) {
             return;
         }
-        if ("url".equals(localName) && "urlset".equals(currentElementParent())) {
-            maybeAddSiteMapUrl();
+        if ("url".equals(localName)) {
+            if ("urlset".equals(currentElementParent())) {
+                maybeAddSiteMapUrl();
+            }
         } else if ("urlset".equals(localName)) {
             sitemap.setProcessed(true);
+        } else if ("loc".equals(localName)) {
+            loc = getAndResetCharacterBuffer();
+        } else if ("changefreq".equals(localName)) {
+            changeFreq = getAndResetCharacterBuffer();
+        } else if ("lastmod".equals(localName)) {
+            lastMod = getAndResetCharacterBuffer();
+        } else if ("priority".equals(localName)) {
+            priority = getAndResetCharacterBuffer();
         }
     }
 
+    @Override
     public void characters(char[] ch, int start, int length) throws SAXException {
         if (isExtensionNamespace(currentElementNamespace)) {
             ExtensionHandler eh = getExtensionHandler(currentElementNamespace);
@@ -131,25 +147,27 @@ class XMLHandler extends DelegatorHandler {
         } else if (isStrictNamespace() && !currentElementNamespaceIsValid) {
             return;
         }
-        String localName = super.currentElement();
-        String value = String.valueOf(ch, start, length);
-        if ("loc".equals(localName) || "url".equals(localName)) {
-            loc.append(value);
-        } else if ("changefreq".equals(localName)) {
-            changeFreq = value;
-        } else if ("lastmod".equals(localName)) {
-            lastMod = value;
-        } else if ("priority".equals(localName)) {
-            priority = value;
+        String localName = currentElement();
+        if ("loc".equals(localName) || "url".equals(localName) || "changefreq".equals(localName) || "lastmod".equals(localName) || "priority".equals(localName)) {
+            appendCharacterBuffer(ch, start, length);
         }
     }
 
+    @Override
     public AbstractSiteMap getSiteMap() {
         return sitemap;
     }
 
     private void maybeAddSiteMapUrl() {
-        String value = stripAllBlank(loc);
+        String value = null;
+        if (loc != null) {
+            value = stripAllBlank(loc);
+        } else if ("loc".equals(currentElement())) {
+            value = getAndResetCharacterBuffer();
+        }
+        if (value == null || isAllBlank(value)) {
+            return;
+        }
         try {
             // check that the value is a valid URL
             URL locURL = new URL(value);
@@ -171,7 +189,7 @@ class XMLHandler extends DelegatorHandler {
             LOG.debug("Bad url: [{}]", value);
             LOG.trace("Can't create an entry with a bad URL", e);
         } finally {
-            loc = new StringBuilder();
+            loc = null;
             lastMod = null;
             changeFreq = null;
             priority = null;
@@ -220,10 +238,12 @@ class XMLHandler extends DelegatorHandler {
         }
     }
 
+    @Override
     public void error(SAXParseException e) throws SAXException {
         maybeAddSiteMapUrl();
     }
 
+    @Override
     public void fatalError(SAXParseException e) throws SAXException {
         maybeAddSiteMapUrl();
     }
diff --git a/src/main/java/crawlercommons/sitemaps/sax/XMLIndexHandler.java b/src/main/java/crawlercommons/sitemaps/sax/XMLIndexHandler.java
index 10640de..8de44bf 100644
--- a/src/main/java/crawlercommons/sitemaps/sax/XMLIndexHandler.java
+++ b/src/main/java/crawlercommons/sitemaps/sax/XMLIndexHandler.java
@@ -54,7 +54,7 @@ import crawlercommons.sitemaps.AbstractSiteMap.SitemapType;
 class XMLIndexHandler extends DelegatorHandler {
 
     private SiteMapIndex sitemap;
-    private StringBuilder loc;
+    private String loc;
     private boolean locClosed;
     private Date lastMod;
     private int i = 0;
@@ -63,17 +63,17 @@ class XMLIndexHandler extends DelegatorHandler {
         super(elementStack, strict);
         sitemap = new SiteMapIndex(url);
         sitemap.setType(SitemapType.INDEX);
-        loc = new StringBuilder();
     }
 
+    @Override
     public void startElement(String uri, String localName, String qName, Attributes attributes) throws SAXException {
         // flush any unclosed or missing <sitemap> element
-        if (loc.length() > 0 && ("loc".equals(localName) || "sitemap".equals(localName))) {
+        if (loc != null && loc.length() > 0 && ("loc".equals(localName) || "sitemap".equals(localName))) {
             if (!isAllBlank(loc)) {
                 maybeAddSiteMap();
                 return;
             }
-            loc = new StringBuilder();
+            loc = null;
             if ("sitemap".equals(localName)) {
                 // reset also attributes
                 locClosed = false;
@@ -82,41 +82,55 @@ class XMLIndexHandler extends DelegatorHandler {
         }
     }
 
+    @Override
     public void endElement(String uri, String localName, String qName) throws SAXException {
         if (isStrictNamespace() && !isAcceptedNamespace(uri)) {
             return;
         }
-        if ("sitemap".equals(currentElement())) {
+        if ("sitemap".equals(localName)) {
+            if (!locClosed) {
+                // closing </sitemap> without closed </loc>
+                // try text in <sitemap> as <loc>
+                loc = getAndResetCharacterBuffer();
+            }
             maybeAddSiteMap();
-        } else if ("sitemapindex".equals(currentElement())) {
+        } else if ("sitemapindex".equals(localName)) {
             sitemap.setProcessed(true);
-        } else if ("loc".equals(currentElement())) {
+        } else if ("lastmod".equals(localName)) {
+            String value = getAndResetCharacterBuffer();
+            if (value != null) {
+                lastMod = SiteMap.convertToDate(value);
+            }
+        } else if ("loc".equals(localName)) {
+            loc = getAndResetCharacterBuffer();
             locClosed = true;
         }
     }
 
+    @Override
     public void characters(char[] ch, int start, int length) throws SAXException {
         String localName = super.currentElement();
-        String value = String.valueOf(ch, start, length);
-        if ("loc".equals(localName)) {
-            loc.append(value);
-        } else if ("lastmod".equals(localName)) {
-            lastMod = SiteMap.convertToDate(value);
-        } else {
-            value = stripAllBlank(value);
-            if (!value.isEmpty() && !locClosed) {
-                // try non-whitespace text content as loc
-                // when no loc element has been specified
-                loc.append(value);
+        if ("loc".equals(localName) || "lastmod".equals(localName)) {
+            appendCharacterBuffer(ch, start, length);
+        } else if (!locClosed) {
+            // try non-whitespace text content as loc
+            // when no loc element has been specified
+            String value = stripAllBlank(String.valueOf(ch, start, length));
+            if (!value.isEmpty()) {
+                appendCharacterBuffer(value);
             }
         }
     }
 
+    @Override
     public AbstractSiteMap getSiteMap() {
         return sitemap;
     }
 
     private void maybeAddSiteMap() {
+        if (loc == null) {
+            return;
+        }
         String value = stripAllBlank(loc);
         try {
             // check that the value is a valid URL
@@ -128,15 +142,17 @@ class XMLIndexHandler extends DelegatorHandler {
             LOG.trace("Don't create an entry with a bad URL", e);
             LOG.debug("Bad url: [{}]", value);
         }
-        loc = new StringBuilder();
+        loc = null;
         locClosed = false;
         lastMod = null;
     }
 
+    @Override
     public void error(SAXParseException e) throws SAXException {
         maybeAddSiteMap();
     }
 
+    @Override
     public void fatalError(SAXParseException e) throws SAXException {
         maybeAddSiteMap();
     }
diff --git a/src/main/java/crawlercommons/sitemaps/sax/extension/LinksHandler.java b/src/main/java/crawlercommons/sitemaps/sax/extension/LinksHandler.java
index 951f549..91e3014 100644
--- a/src/main/java/crawlercommons/sitemaps/sax/extension/LinksHandler.java
+++ b/src/main/java/crawlercommons/sitemaps/sax/extension/LinksHandler.java
@@ -53,13 +53,4 @@ public class LinksHandler extends ExtensionHandler {
             }
         }
     }
-
-    @Override
-    public void endElement(String uri, String localName, String qName) throws SAXException {
-    }
-
-    @Override
-    public void characters(char[] ch, int start, int length) throws SAXException {
-    }
-
 }
diff --git a/src/main/java/crawlercommons/sitemaps/sax/extension/MobileHandler.java b/src/main/java/crawlercommons/sitemaps/sax/extension/MobileHandler.java
index 3ab8424..60e52f2 100644
--- a/src/main/java/crawlercommons/sitemaps/sax/extension/MobileHandler.java
+++ b/src/main/java/crawlercommons/sitemaps/sax/extension/MobileHandler.java
@@ -44,13 +44,6 @@ public class MobileHandler extends ExtensionHandler {
     }
 
     @Override
-    public void endElement(String uri, String localName, String qName) throws SAXException {
-    }
-
-    @Override
-    public void characters(char[] ch, int start, int length) throws SAXException {
-    }
-
     public ExtensionMetadata[] getAttributes() {
         if (mobileElementFound) {
             return mobileAttributes;
@@ -58,6 +51,7 @@ public class MobileHandler extends ExtensionHandler {
         return noMobileAttributes;
     }
 
+    @Override
     public void reset() {
         super.reset();
         mobileElementFound = false;
diff --git a/src/main/java/crawlercommons/sitemaps/sax/extension/VideoHandler.java b/src/main/java/crawlercommons/sitemaps/sax/extension/VideoHandler.java
index 1378785..1243d33 100644
--- a/src/main/java/crawlercommons/sitemaps/sax/extension/VideoHandler.java
+++ b/src/main/java/crawlercommons/sitemaps/sax/extension/VideoHandler.java
@@ -18,14 +18,12 @@ package crawlercommons.sitemaps.sax.extension;
 
 import static crawlercommons.sitemaps.SiteMapParser.LOG;
 
-import java.time.ZonedDateTime;
 import java.util.Map;
 import java.util.TreeMap;
 
 import org.xml.sax.Attributes;
 import org.xml.sax.SAXException;
 
-import crawlercommons.sitemaps.SiteMap;
 import crawlercommons.sitemaps.extension.ExtensionMetadata;
 import crawlercommons.sitemaps.extension.VideoAttributes;
 import crawlercommons.sitemaps.extension.VideoAttributes.VideoPrice;
@@ -150,8 +148,7 @@ public class VideoHandler extends ExtensionHandler {
             }
             currAttr.setDuration(duration);
         } else if ("expiration_date".equals(localName)) {
-            ZonedDateTime dateTime = SiteMap.convertToZonedDateTime(value);
-            currAttr.setExpirationDate(dateTime);
+            currAttr.setExpirationDate(getDateValue(value));
         } else if ("rating".equals(localName)) {
             currAttr.setRating(getFloatValue(value));
         } else if ("view_count".equals(localName)) {


Test patch:
diff --git a/src/test/java/crawlercommons/sitemaps/SiteMapParserTest.java b/src/test/java/crawlercommons/sitemaps/SiteMapParserTest.java
index 09332fe..480b708 100644
--- a/src/test/java/crawlercommons/sitemaps/SiteMapParserTest.java
+++ b/src/test/java/crawlercommons/sitemaps/SiteMapParserTest.java
@@ -31,9 +31,12 @@ import java.io.IOException;
 import java.io.InputStream;
 import java.net.URL;
 import java.nio.charset.StandardCharsets;
+import java.time.ZonedDateTime;
+import java.time.temporal.ChronoField;
 import java.util.ArrayList;
 import java.util.Iterator;
 import java.util.List;
+import java.util.Locale;
 
 import org.apache.commons.io.IOUtils;
 import org.junit.After;
@@ -72,6 +75,7 @@ public class SiteMapParserTest {
                         .append(" </sitemap>\n") //
                         .append("<sitemap>\n") //
                         .append("  <loc>http://www.example.com/dynsitemap?date=now&amp;all=true</loc>\n") //
+                        .append("  <lastmod>2004-10-01T18:23:17&#43;00:00</lastmod>\n") //
                         .append(" </sitemap>\n") //
                         .append("<sitemap>\n") //
                         .append("  <loc>http://www.example.com/dynsitemap<![CDATA[?date=lastyear&all=false]]></loc>\n") //
@@ -102,6 +106,10 @@ public class SiteMapParserTest {
         currentSiteMap = smi.getSitemap(new URL("http://www.example.com/dynsitemap?date=now&all=true"));
         assertNotNull("<loc> with entities not found", currentSiteMap);
         assertEquals("http://www.example.com/dynsitemap?date=now&all=true", currentSiteMap.getUrl().toString());
+        // test <lastmodified> containing a character entity - the input is
+        // passed in chunks to the characters(...) method in the XMLIndexHandler
+        assertNotNull("<lastmod> containing entities", currentSiteMap.getLastModified());
+        assertEquals("<lastmod> containing entities", SiteMap.convertToDate("2004-10-01T18:23:17+00:00"), currentSiteMap.getLastModified());
 
         currentSiteMap = smi.getSitemap(new URL("http://www.example.com/dynsitemap?date=lastyear&all=false"));
         assertNotNull("<loc> with CDATA not found", currentSiteMap);
@@ -216,7 +224,7 @@ public class SiteMapParserTest {
 
         SiteMapURL[] found = sm.getSiteMapUrls().toArray(new SiteMapURL[5]);
         for (int i = 0; i < found.length; i++) {
-            assertEquals(SITEMAP_URLS[i].replaceAll("&amp;", "&"), found[i].getUrl().toExternalForm());
+            validateSiteMapUrl(i, found[i]);
         }
     }
 
@@ -235,7 +243,7 @@ public class SiteMapParserTest {
             assertEquals(5, sm.getSiteMapUrls().size());
             SiteMapURL[] found = sm.getSiteMapUrls().toArray(new SiteMapURL[5]);
             for (int i = 0; i < found.length; i++) {
-                assertEquals(SITEMAP_URLS[i].replaceAll("&amp;", "&"), found[i].getUrl().toExternalForm());
+                validateSiteMapUrl(i, found[i]);
             }
         }
     }
@@ -439,7 +447,11 @@ public class SiteMapParserTest {
 
         SiteMap sm = (SiteMap) parser.parseSiteMap(content, url);
         assertEquals(1, sm.getSiteMapUrls().size());
-        assertEquals(new URL("http://example.org/2003/12/13/atom03"), sm.getSiteMapUrls().iterator().next().getUrl());
+        SiteMapURL smu = sm.getSiteMapUrls().iterator().next();
+        assertEquals(new URL("http://example.org/2003/12/13/atom03"), smu.getUrl());
+        // test for <updated>2003-12-13T18:30:02Z</updated>
+        assertNotNull(smu.getLastModified());
+        assertEquals(12, ZonedDateTime.ofInstant(smu.getLastModified().toInstant(), AbstractSiteMap.TIME_ZONE_UTC).get(ChronoField.MONTH_OF_YEAR));
     }
 
     @Test
@@ -451,7 +463,12 @@ public class SiteMapParserTest {
         SiteMap sm = (SiteMap) parser.parseSiteMap(content, url);
         assertEquals(4, sm.getSiteMapUrls().size());
         Iterator<SiteMapURL> it = sm.getSiteMapUrls().iterator();
-        assertEquals(new URL("https://www.example.com/blog/post/1"), it.next().getUrl());
+        SiteMapURL smu = it.next();
+        assertEquals(new URL("https://www.example.com/blog/post/1"), smu.getUrl());
+        // test for <pubDate>Sun, 06 Sep 2009 16:20:00 +0000</pubDate>
+        assertNotNull(smu.getLastModified());
+        assertEquals(9, ZonedDateTime.ofInstant(smu.getLastModified().toInstant(), AbstractSiteMap.TIME_ZONE_UTC).get(ChronoField.MONTH_OF_YEAR));
+
         assertEquals(new URL("https://www.example.com/guid.html"), it.next().getUrl());
         assertEquals(new URL("https://www.example.com/foo?q=a&l=en"), it.next().getUrl());
         assertEquals(new URL("https://www.example.com/foo?q=a&l=fr"), it.next().getUrl());
@@ -590,9 +607,14 @@ public class SiteMapParserTest {
         SiteMapParser parser = new SiteMapParser(false);
         String contentType = "text/xml";
         StringBuilder scontent = new StringBuilder(1024);
-        scontent.append("<?xml version=\"1.0\" encoding=\"UTF-8\"?>").append("<urlset xmlns=\"http://www.sitemaps.org/schemas/sitemap/0.9\">").append("<url>").append("<loc>").append("<url>")
-                        .append("<![CDATA[").append("http://jobs.optistaffing.com/EXPERIENCED-DISPATCHER-NEEDED-NOW----Jobs-in-Vancouver-WA/2333221").append("]]>").append("</url>").append("</loc>")
-                        .append("<lastmod>2015-04-28</lastmod>").append("<changefreq>daily</changefreq>").append("</url>").append("</urlset>");
+        scontent.append("<?xml version=\"1.0\" encoding=\"UTF-8\"?>") //
+                        .append("<urlset xmlns=\"http://www.sitemaps.org/schemas/sitemap/0.9\">") //
+                        .append("<url><loc><url>") //
+                        .append("<![CDATA[http://jobs.optistaffing.com/EXPERIENCED-DISPATCHER-NEEDED-NOW----Jobs-in-Vancouver-WA/2333221]]>") //
+                        .append("</url>").append("</loc>") //
+                        .append("<lastmod>2015-04-28</lastmod>") //
+                        .append("<changefreq>daily</changefreq>") //
+                        .append("</url></urlset>");
 
         byte[] content = scontent.toString().getBytes(UTF_8);
 
@@ -645,13 +667,21 @@ public class SiteMapParserTest {
      */
     private byte[] getXMLSitemapAsBytes() {
         StringBuilder scontent = new StringBuilder(1024);
-        scontent.append("<?xml version=\"1.0\" encoding=\"UTF-8\"?>").append("<urlset xmlns=\"http://www.sitemaps.org/schemas/sitemap/0.9\">");
-        scontent.append("<url>  <loc>").append(SITEMAP_URLS[0]).append("</loc>  <lastmod>2005-01-01</lastmod>").append("  <changefreq>monthly</changefreq>").append("  <priority>0.8</priority>")
-                        .append("</url>");
-        scontent.append("<url>  <loc>").append(SITEMAP_URLS[1]).append("</loc>  <changefreq>weekly</changefreq>").append("</url>");
-        scontent.append("<url>  <loc>").append(SITEMAP_URLS[2]).append("</loc>  <lastmod>2004-12-23</lastmod>").append("  <changefreq>weekly</changefreq>").append("</url>");
-        scontent.append("<url>  <loc>").append(SITEMAP_URLS[3]).append("</loc>  <lastmod>2004-12-23T18:00:15+00:00</lastmod>").append("  <priority>0.3</priority>").append("</url>");
-        scontent.append("<url>  <loc><url><![CDATA[").append(SITEMAP_URLS[4]).append("]]></url></loc>  <lastmod>2004-11-23</lastmod>").append("</url>");
+        scontent.append("<?xml version=\"1.0\" encoding=\"UTF-8\"?>") //
+                        .append("<urlset xmlns=\"http://www.sitemaps.org/schemas/sitemap/0.9\">");
+        for (String[] surl : SITEMAP_URLS) {
+            scontent.append(" <url>\n  <loc>").append(surl[0]).append("</loc>\n");
+            if (surl[1] != null) {
+                scontent.append("  <lastmod>").append(surl[1]).append("</lastmod>\n");
+            }
+            if (surl[2] != null) {
+                scontent.append("  <changefreq>").append(surl[2]).append("</changefreq>\n");
+            }
+            if (surl[3] != null) {
+                scontent.append("  <priority>").append(surl[3]).append("</priority>\n");
+            }
+            scontent.append(" </url>\n");
+        }
         scontent.append("</urlset>");
 
         return scontent.toString().getBytes(UTF_8);
@@ -671,10 +701,23 @@ public class SiteMapParserTest {
         return IOUtils.toByteArray(is);
     }
 
-    private static String[] SITEMAP_URLS = { "http://www.example.com/", //
-                    "http://www.example.com/catalog?item=12&amp;desc=vacation_hawaii", //
-                    "http://www.example.com/catalog?item=73&amp;desc=vacation_new_zealand", //
-                    "http://www.example.com/catalog?item=74&amp;desc=vacation_newfoundland", //
-                    "http://www.example.com/catalog?item=83&desc=vacation_usa" };
+    private static String[][] SITEMAP_URLS = { { "http://www.example.com/", "2005-01-01", "monthly", "0.8" },
+                    { "http://www.example.com/catalog?item=12&amp;desc=vacation_hawaii", null, "weekly", null }, //
+                    { "http://www.example.com/catalog?item=73&amp;desc=vacation_new_zealand", "2004-12-23", "weekly", null }, //
+                    { "http://www.example.com/catalog?item=74&amp;desc=vacation_newfoundland", "2004-12-23T18:00:15&#43;00:00", null, "0.3" }, //
+                    { "http://www.example.com/catalog?item=83&amp;desc=vacation_usa", "2004-11-23", null, null } };
+
+    private static void validateSiteMapUrl(int i, SiteMapURL u) {
+        assertEquals(SITEMAP_URLS[i][0].replaceAll("&amp;", "&"), u.getUrl().toExternalForm());
+        if (SITEMAP_URLS[i][1] != null) {
+            assertNotNull("No <lastmod>" + SITEMAP_URLS[i][1] + "</lastmod>", u.getLastModified());
+        }
+        if (SITEMAP_URLS[i][2] != null) {
+            assertEquals("Wrong <changefreq>" + SITEMAP_URLS[i][2] + "</changefreq>", SITEMAP_URLS[i][2], u.getChangeFrequency().toString().toLowerCase(Locale.ROOT));
+        }
+        if (SITEMAP_URLS[i][3] != null) {
+            assertEquals("Wrong <priority>" + SITEMAP_URLS[i][3] + "</priority>", Double.parseDouble(SITEMAP_URLS[i][3]), u.getPriority(), .0001);
+        }
+    }
 
 }