joerghoh commented on code in PR #53:
URL: 
https://github.com/apache/sling-org-apache-sling-engine/pull/53#discussion_r1847930730


##########
src/main/java/org/apache/sling/engine/impl/SlingHttpServletResponseImpl.java:
##########
@@ -339,28 +360,87 @@ private Optional<String> 
checkContentTypeOverride(@Nullable String contentType)
     }
 
     /**
-     * Retrieves the message to log when the 'Content-Type' header is changed 
via an include.
+     * Finds unmatched TIMER_START messages in a log of messages.
+     *
+     * @return a string containing the unmatched TIMER_START messages
+     */
+    private String findUnmatchedTimerStarts() {
+        Iterator<String> messages = 
requestData.getRequestProgressTracker().getMessages();
+        List<String> unmatchedStarts = new ArrayList<>();
+        Deque<String> timerDeque = new ArrayDeque<>();
+
+        Pattern startPattern = Pattern.compile(REGEX_TIMER_START);
+        Pattern endPattern = Pattern.compile(REGEX_TIMER_END);
+
+        while (messages.hasNext()) {
+            String message = messages.next();
+            Matcher startMatcher = startPattern.matcher(message);
+            Matcher endMatcher = endPattern.matcher(message);
+
+            // use a Deque to keep track of the timers that have been started. 
When
+            // an end timer is found, it is compared to the top of the deque. 
If they match,
+            // the timer is removed from the deque. If they don't match, the 
timer is added
+            // to the list of unmatched starts. As the deque is a LIFO data 
structure, the
+            // last timer that was started will be the first one to be ended. 
There is no
+            // Start1, Start2, End1 scenario, without an End2 in between.
+            if (startMatcher.find()) {
+                timerDeque.push(startMatcher.group(1));
+            } else if (endMatcher.find()) {
+                String endTimer = endMatcher.group(1);
+                if (!timerDeque.isEmpty() && 
timerDeque.peek().equals(endTimer)) {
+                    timerDeque.pop();
+                } else {
+                    unmatchedStarts.add(endTimer);
+                }
+            }
+        }
+
+        // ignore the first element, as it will never have a matching end, as 
it is the
+        // first timer started and is not finished processing
+        while (timerDeque.size() > 1) {
+            unmatchedStarts.add(timerDeque.pop());
+        }
+
+        return unmatchedStarts.toString().trim();
+    }
+
+    /**
+     * Retrieves the message to log when the 'Content-Type' header is changed 
via an
+     * include.
+     *
      * @param currentContentType the current 'Content-Type' header
      * @param setContentType the 'Content-Type' header that is being set
      */
     private String getMessage(@Nullable String currentContentType, @Nullable 
String setContentType) {
+        String unmatchedStartTimers = findUnmatchedTimerStarts();
+        String allMessages = StreamSupport.stream(
+                        Spliterators.spliteratorUnknownSize(
+                                
requestData.getRequestProgressTracker().getMessages(), 0),
+                        false)
+                .collect(Collectors.joining(System.lineSeparator()));
         if (!isCheckContentTypeOnInclude()) {
             return String.format(
                     "Servlet %s tried to override the 'Content-Type' header 
from '%s' to '%s'. This is a violation of "
                             + "the RequestDispatcher.include() contract - "
-                            + 
"https://jakarta.ee/specifications/servlet/4.0/apidocs/javax/servlet/requestdispatcher#include-javax.servlet.ServletRequest-javax.servlet.ServletResponse-.";,
-                    requestData.getActiveServletName(), currentContentType, 
setContentType);
+                            + 
"https://jakarta.ee/specifications/servlet/4.0/apidocs/javax/servlet/requestdispatcher#include-javax.servlet.ServletRequest-javax.servlet.ServletResponse-.
 , Including scripts: %s. All RequestProgressTracker messages: %s",
+                    requestData.getActiveServletName(),
+                    currentContentType,
+                    setContentType,
+                    unmatchedStartTimers,
+                    allMessages);
         }
         return String.format(
                 "Servlet %s tried to override the 'Content-Type' header from 
'%s' to '%s', however the"
                         + " %s forbids this via the %s configuration property. 
This is a violation of the "
                         + "RequestDispatcher.include() contract - "
-                        + 
"https://jakarta.ee/specifications/servlet/4.0/apidocs/javax/servlet/requestdispatcher#include-javax.servlet.ServletRequest-javax.servlet.ServletResponse-.";,
+                        + 
"https://jakarta.ee/specifications/servlet/4.0/apidocs/javax/servlet/requestdispatcher#include-javax.servlet.ServletRequest-javax.servlet.ServletResponse-.
 , Including scripts: %s. All RequestProgressTracker messages: %s",

Review Comment:
   +1 for option 3. 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to