[ofbiz-framework] branch trunk updated: Improved: Well-formed html in ftl template (OFBIZ-11996)

[James Yong-2]

This is an automated email from the ASF dual-hosted git repository.

jamesyong pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/ofbiz-framework.git


The following commit(s) were added to refs/heads/trunk by this push:
     new 161c0c0  Improved: Well-formed html in ftl template (OFBIZ-11996)
161c0c0 is described below

commit 161c0c0c05ad33474779706cf8bf1d548528f817
Author: James Yong <[hidden email]>
AuthorDate: Thu Sep 10 17:12:36 2020 +0800

    Improved: Well-formed html in ftl template (OFBIZ-11996)
   
    refactoring + add test case
---
 .../java/org/apache/ofbiz/base/util/UtilHtml.java  | 75 ++++++++++++----------
 .../org/apache/ofbiz/base/util/UtilHtmlTest.java   | 13 +++-
 .../org/apache/ofbiz/widget/model/HtmlWidget.java  | 10 +--
 3 files changed, 56 insertions(+), 42 deletions(-)

diff --git a/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilHtml.java b/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilHtml.java
index 39f185e..94f72c3 100644
--- a/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilHtml.java
+++ b/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilHtml.java
@@ -29,6 +29,7 @@ import javax.xml.stream.events.StartElement;
 import javax.xml.stream.events.XMLEvent;
 import java.io.ByteArrayInputStream;
 import java.nio.charset.StandardCharsets;
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
 import java.util.Stack;
@@ -59,51 +60,57 @@ public final class UtilHtml {
     /**
      *
      * @param content
-     * @param locationInfo for printing location information
-     * @return true if there is error
+     * @return list of errors
      */
-    public static boolean hasUnclosedTag(String content, String locationInfo) {
+    public static List<String> hasUnclosedTag(String content) {
         XMLInputFactory inputFactory = XMLInputFactory.newInstance();
         XMLEventReader eventReader = null;
+        List<String> errorList = new ArrayList<>();
         try {
+            // wrap with template tag as some content contains multiple root
             eventReader = inputFactory.createXMLEventReader(
-                    new ByteArrayInputStream(content.getBytes(StandardCharsets.UTF_8)), "utf-8");
-        } catch (XMLStreamException e) {
-            Debug.logError(e.getMessage(), MODULE);
-            return true;
-        }
-
-        Stack<StartElement> stack = new Stack<StartElement>();
-        boolean hasError = false;
-        while (eventReader.hasNext()) {
-            try {
-                XMLEvent event = eventReader.nextEvent();
-                if (event.isStartElement()) {
-                    StartElement startElement = event.asStartElement();
-                    stack.push(startElement);
-                }
-                if (event.isEndElement()) {
-                    EndElement endElement = event.asEndElement();
-                    stack.pop();
-                }
-            } catch (XMLStreamException e) {
-                if (!stack.isEmpty()) {
-                    StartElement startElement = stack.pop();
-                    String elementName = startElement.getName().getLocalPart();
-                    if (Arrays.stream(TAG_SHOULD_CLOSE_LIST).anyMatch(elementName::equals)) {
-                        hasError = true;
-                        UtilHtml.logFormattedError(content, locationInfo, e.getMessage(), MODULE);
+                    new ByteArrayInputStream(("<template>" + content + "</template>").getBytes(StandardCharsets.UTF_8)),
+                    "utf-8");
+            Stack<StartElement> stack = new Stack<StartElement>();
+            while (eventReader.hasNext()) {
+                try {
+                    XMLEvent event = eventReader.nextEvent();
+                    if (event.isStartElement()) {
+                        StartElement startElement = event.asStartElement();
+                        stack.push(startElement);
                     }
-                } else {
-                    UtilHtml.logFormattedError(content, locationInfo, e.getMessage(), MODULE);
+                    if (event.isEndElement()) {
+                        EndElement endElement = event.asEndElement();
+                        stack.pop();
+                    }
+                } catch (XMLStreamException e) {
+                    if (!stack.isEmpty()) {
+                        StartElement startElement = stack.pop();
+                        String elementName = startElement.getName().getLocalPart();
+                        if (Arrays.stream(TAG_SHOULD_CLOSE_LIST).anyMatch(elementName::equals)) {
+                            errorList.add(e.getMessage());
+                        }
+                    } else {
+                        errorList.add(e.getMessage());
+                    }
+                    break;
+                }
+            }
+        } catch (XMLStreamException e) {
+            errorList.add(e.getMessage());
+        } finally {
+            if (eventReader != null) {
+                try {
+                    eventReader.close();
+                } catch (XMLStreamException e) {
+                    // do nothing
                 }
-                break;
             }
         }
-        return hasError;
+        return errorList;
     }
 
     public static void logFormattedError(String content, String location, String error, String module) {
-        Debug.logError("[Parsing " + location + "]" + error, module);
+        Debug.logError("[Parsing " + location + "] " + error, module);
     }
 }
diff --git a/framework/base/src/test/java/org/apache/ofbiz/base/util/UtilHtmlTest.java b/framework/base/src/test/java/org/apache/ofbiz/base/util/UtilHtmlTest.java
index e5a132e..347447c 100644
--- a/framework/base/src/test/java/org/apache/ofbiz/base/util/UtilHtmlTest.java
+++ b/framework/base/src/test/java/org/apache/ofbiz/base/util/UtilHtmlTest.java
@@ -20,15 +20,22 @@ package org.apache.ofbiz.base.util;
 
 import org.junit.Test;
 
-import javax.xml.stream.XMLStreamException;
+import java.util.List;
 
 import static org.junit.Assert.assertEquals;
 
 public class UtilHtmlTest {
 
     @Test
-    public void parseHtmlFragment_1() throws XMLStreamException {
-        assertEquals(true, UtilHtml.hasUnclosedTag("<div><div></div>", "location.ftl"));
+    public void parseHtmlFragment_unclosedDiv() {
+        List<String> errorList = UtilHtml.hasUnclosedTag("<div><div></div>");
+        assertEquals(true, errorList.get(0).contains("Unexpected close tag"));
+    }
+
+    @Test
+    public void parseHtmlFragment_multiRoot() {
+        List<String> errorList = UtilHtml.hasUnclosedTag("<div></div><div></div>");
+        assertEquals(0, errorList.size());
     }
 }
 
diff --git a/framework/widget/src/main/java/org/apache/ofbiz/widget/model/HtmlWidget.java b/framework/widget/src/main/java/org/apache/ofbiz/widget/model/HtmlWidget.java
index b3368e0..bd21c03 100644
--- a/framework/widget/src/main/java/org/apache/ofbiz/widget/model/HtmlWidget.java
+++ b/framework/widget/src/main/java/org/apache/ofbiz/widget/model/HtmlWidget.java
@@ -58,7 +58,6 @@ import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Stack;
-import java.util.function.Consumer;
 
 /**
  * Widget Library - Screen model HTML class.
@@ -271,12 +270,13 @@ public class HtmlWidget extends ModelScreenWidget {
 
             if (Debug.verboseOn()) {
                 // check for unclosed tags
-                boolean hasError = false; //UtilHtml.hasUnclosedTag(data, location);
-                if (!hasError) {
+                List<String> errorList = UtilHtml.hasUnclosedTag(data);
+                if (UtilValidate.isNotEmpty(errorList)) {
+                    errorList.forEach(a -> UtilHtml.logFormattedError(data, location, a, MODULE));
+                    // check with JSoup Html Parser
                     List<ParseError> errList = UtilHtml.validateHtmlFragmentWithJSoup(data);
                     if (UtilValidate.isNotEmpty(errList)) {
-                        Consumer<ParseError> logError = a -> UtilHtml.logFormattedError(data, location, a.toString(), MODULE);
-                        errList.forEach(logError);
+                        errList.forEach(a -> UtilHtml.logFormattedError(data, location, a.toString(), MODULE));
                     }
                 }
             }