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


##########
src/main/java/org/apache/sling/engine/impl/SlingJakartaHttpServletResponseImpl.java:
##########
@@ -375,9 +375,81 @@ public void setContentType(final String type) {
         }
     }
 
+    @Override
+    public void setCharacterEncoding(final String charset) {
+        boolean isCommitedDueToSendErrorOrRedirect = this.isCommitted()
+                && (CommitReason.SEND_ERROR == this.committedReason
+                        || CommitReason.SEND_REDIRECT == this.committedReason);
+        if (isCommitedDueToSendErrorOrRedirect || !isInclude()) {
+            super.setCharacterEncoding(charset);
+            return;
+        }
+        final Optional<String> message = 
checkCharacterEncodingOverride(charset);
+        if (message.isPresent()) {
+            if (isCheckContentTypeOnInclude()) {
+                requestData.getRequestProgressTracker().log("ERROR: " + 
message.get());
+                LOG.error(CALL_STACK_MESSAGE + getCurrentStackTrace());
+                throw new ContentTypeChangeException(message.get());
+            }
+            if (isProtectHeadersOnInclude()) {
+                LOG.error(message.get());
+                LOG.error(CALL_STACK_MESSAGE + getCurrentStackTrace());
+                requestData.getRequestProgressTracker().log("ERROR: " + 
message.get());
+                return;
+            }
+            LOG.warn(message.get());
+            LOG.warn(CALL_STACK_MESSAGE + getCurrentStackTrace());
+            requestData.getRequestProgressTracker().log("WARN: " + 
message.get());
+            super.setCharacterEncoding(charset);
+        } else {
+            super.setCharacterEncoding(charset);
+        }
+    }
+
+    @Override
+    public void setCharacterEncoding(final Charset charset) {
+        // funnel the Charset variant through the checked String variant so
+        // that the include protections cannot be bypassed via this method
+        this.setCharacterEncoding(charset == null ? null : charset.name());
+    }
+
+    /**
+     * Checks if the response character encoding is being changed by an include
+     * and provides a message to log if it is. Changing the character encoding
+     * changes the charset parameter of the 'Content-Type' header and is
+     * therefore subject to the same include protections as
+     * {@link #setContentType(String)}.
+     *
+     * @param charset the character encoding that is being set
+     * @return an optional message to log
+     */
+    protected Optional<String> checkCharacterEncodingOverride(@Nullable String 
charset) {
+        // A previously detected violation must not disable the check itself -
+        // otherwise the second and any later override attempt within the same
+        // request would pass unchecked even though the first one was blocked.
+        final boolean isFirstViolation =
+                
requestData.getSlingRequestProcessor().getContentTypeHeaderState() != 
ContentTypeHeaderState.VIOLATED;
+        final String currentCharset = getCharacterEncoding();
+        if (charset != null && charset.equalsIgnoreCase(currentCharset)) {

Review Comment:
   the code now handles aliased charsets properly.



-- 
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