Skip to content

Commit

Permalink
BACKLOG-22437: Refactor code to remove some complexities and bad log …
Browse files Browse the repository at this point in the history 
…formatting
  • Loading branch information
@cedmail
cedmail committed Feb 28, 2024
1 parent 3b9de77 commit d4b2ddb
Show file tree
Hide file tree
Showing 10 changed files with 174 additions and 159 deletions.
18 changes: 9 additions & 9 deletions src/main/java/org/jahia/modules/htmlfiltering/HTMLFilteringInterface.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,21 +20,21 @@

public interface HTMLFilteringInterface {

public static String DEFAULT_POLICY_KEY = "default";
String DEFAULT_POLICY_KEY = "default";

public String getCKEditor5Config(String siteKey);
String getCKEditor5Config(String siteKey);

public String getCKEditor4Config(String siteKey);
String getCKEditor4Config(String siteKey);

public PolicyFactory getOwaspPolicyFactory(String siteKey);
PolicyFactory getOwaspPolicyFactory(String siteKey);

public PolicyFactory getDefaultOwaspPolicyFactory();
PolicyFactory getDefaultOwaspPolicyFactory();

public PolicyFactory getMergedOwaspPolicyFactory(String... siteKeys);
PolicyFactory getMergedOwaspPolicyFactory(String... siteKeys);

public JSONObject getMergedJSONPolicy(String... siteKeys);
JSONObject getMergedJSONPolicy(String... siteKeys);

public boolean configExists(String siteKey);
boolean configExists(String siteKey);

public boolean htmlSanitizerDryRun(String siteKey);
boolean htmlSanitizerDryRun(String siteKey);
}
108 changes: 56 additions & 52 deletions src/main/java/org/jahia/modules/htmlfiltering/HtmlFilteringInterceptor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import org.jahia.services.content.decorator.JCRSiteNode;
import org.jahia.services.content.interceptor.BaseInterceptor;
import org.jahia.services.content.nodetypes.ExtendedPropertyDefinition;
import org.jetbrains.annotations.NotNull;
import org.osgi.service.component.annotations.Activate;
import org.osgi.service.component.annotations.Component;
import org.osgi.service.component.annotations.Deactivate;
Expand All @@ -34,11 +35,8 @@
import javax.annotation.Nullable;
import javax.jcr.RepositoryException;
import javax.jcr.Value;
import javax.jcr.ValueFormatException;
import javax.jcr.lock.LockException;
import javax.jcr.nodetype.ConstraintViolationException;
import javax.jcr.version.VersionException;
import java.util.Collections;
import java.util.Objects;

@Component(immediate = true)
public class HtmlFilteringInterceptor extends BaseInterceptor {
Expand Down Expand Up @@ -66,26 +64,14 @@ public void setJcrStoreService(JCRStoreService jcrStoreService) {

@Override
public Value beforeSetValue(JCRNodeWrapper node, String name,
ExtendedPropertyDefinition definition, Value originalValue)
throws ValueFormatException, VersionException, LockException,
ConstraintViolationException, RepositoryException {
ExtendedPropertyDefinition definition, Value originalValue)
throws RepositoryException {

String content = originalValue.getString();
if (StringUtils.isEmpty(content) || !content.contains("<")) {
if (logger.isDebugEnabled()) {
logger.debug("The value does not contain any HTML tags. Skip filtering.");
}
return originalValue;
}
if (valueIsEmpty(originalValue)) return originalValue;

JCRSiteNode resolveSite = node.getResolveSite();
if (!resolveSite.isHtmlMarkupFilteringEnabled()) {
return originalValue;
}

HTMLFilteringInterface filteringConfig = BundleUtils.getOsgiService(HTMLFilteringInterface.class, null);

if (filteringConfig == null) {
if (!resolveSite.isHtmlMarkupFilteringEnabled() || filteringConfig == null) {
return originalValue;
}

Expand All @@ -95,66 +81,84 @@ public Value beforeSetValue(JCRNodeWrapper node, String name,
return originalValue;
}

Value modifiedValue = originalValue;

if (logger.isDebugEnabled()) {
logger.debug("Performing HTML tag filtering for " + node.getPath() + "/" + name);
logger.debug("Performing HTML tag filtering for {}/{}", node.getPath(), name);
if (logger.isTraceEnabled()) {
logger.trace("Original value: " + content);
logger.trace("Original value: {}", originalValue.getString());
}
}

String content = originalValue.getString();
String propInfo = node.hasProperty(definition.getName()) ? node.getProperty(definition.getName()).getRealProperty().getPath() : node.getPath();

if (filteringConfig.htmlSanitizerDryRun(resolveSite.getSiteKey())) {
logger.info(String.format("Dry run: Skipping Sanitization of [%s]", propInfo));

policyFactory.sanitize(content, new HtmlChangeListener<Object>() {
@Override
public void discardedTag(@Nullable Object o, String tag) {
logger.info(String.format("Removed tag: %s", tag));
}

@Override
public void discardedAttributes(@Nullable Object o, String tag, String... strings) {
logger.info(String.format("Removed attributes %s for tag %s", String.join(", ", strings), tag));
}
}, null);

return originalValue;
}
if (dryRun(filteringConfig, resolveSite, propInfo, policyFactory, content)) return originalValue;

String result = policyFactory.sanitize(content);

logger.warn(String.format("Sanitized [%s]", propInfo));
logger.warn("Sanitized [{}]", propInfo);

return getModifiedValue(node, preservePlaceholders(result), content, originalValue);
}

@NotNull
private static String preservePlaceholders(String result) {
// Preserve URL context placeholders that might've been encoded by the sanitizer
result = result.replace("%7bmode%7d", "{mode}");
result = result.replace("%7blang%7d", "{lang}");
result = result.replace("%7bworkspace%7d", "{workspace}");
return result;
}

if (result != content && !result.equals(content)) {
modifiedValue = node.getSession().getValueFactory().createValue(result);
private static Value getModifiedValue(JCRNodeWrapper node, String result, String content, Value originalValue) throws RepositoryException {
if (!result.equals(content)) {
Value modifiedValue = node.getSession().getValueFactory().createValue(result);
if (logger.isDebugEnabled()) {
logger.debug("Done filtering of \"unwanted\" HTML tags.");
if (logger.isTraceEnabled()) {
logger.trace("Modified value: " + result);
logger.trace("Modified value: {}", result);
}
}
} else {
return modifiedValue;
} else if (logger.isDebugEnabled()) {
logger.debug("The value does not contain HTML tags that needs to be removed. The content remains unchanged.");
}
return originalValue;
}

private static boolean valueIsEmpty(Value originalValue) throws RepositoryException {
if (StringUtils.isEmpty(originalValue.getString()) || !originalValue.getString().contains("<")) {
if (logger.isDebugEnabled()) {
logger.debug("The value does not contain HTML tags that needs to be removed. The content remains unchanged.");
logger.debug("The value does not contain any HTML tags. Skip filtering.");
}
return true;
}
return false;
}

private static boolean dryRun(HTMLFilteringInterface filteringConfig, JCRSiteNode resolveSite, String propInfo, PolicyFactory policyFactory, String content) {
if (filteringConfig.htmlSanitizerDryRun(resolveSite.getSiteKey())) {
logger.info("Dry run: Skipping Sanitization of [{}]", propInfo);

return modifiedValue;
policyFactory.sanitize(content, new HtmlChangeListener<Object>() {
@Override
public void discardedTag(@Nullable Object o, String tag) {
logger.info(String.format("Removed tag: %s", tag));
}

@Override
public void discardedAttributes(@Nullable Object o, String tag, String... strings) {
logger.info(String.format("Removed attributes %s for tag %s", String.join(", ", strings), tag));
}
}, null);

return true;
}
return false;
}

@Override
public Value[] beforeSetValues(JCRNodeWrapper node, String name,
ExtendedPropertyDefinition definition, Value[] originalValues)
throws ValueFormatException, VersionException, LockException,
ConstraintViolationException, RepositoryException {
ExtendedPropertyDefinition definition, Value[] originalValues)
throws RepositoryException {
Value[] res = new Value[originalValues.length];

for (int i = 0; i < originalValues.length; i++) {
Expand Down
45 changes: 26 additions & 19 deletions src/main/java/org/jahia/modules/htmlfiltering/configuration/HTMLFiltering.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
package org.jahia.modules.htmlfiltering.configuration;

import org.apache.commons.lang.StringUtils;
import org.jahia.api.settings.SettingsBean;
import org.jahia.modules.htmlfiltering.HTMLFilteringInterface;
import org.jahia.modules.htmlfiltering.configuration.parse.Parser;
import org.jahia.modules.htmlfiltering.configuration.parse.PropsToJsonParser;
Expand Down Expand Up @@ -69,9 +68,9 @@ public void updated(String pid, Dictionary<String, ?> dictionary) throws Configu
if (!dictCopy.isEmpty()) {
configs.put(pid, new PropsToJsonParser().parse(dictCopy));
siteKeyToPid.put(siteKey, pid);
logger.info(String.format("Setting htmlFiltering config for site %s: %s", siteKey, configs.get(pid).toString()));
logger.info("Setting htmlFiltering config for site {}: {}", siteKey, configs.get(pid));
} else {
logger.warn(String.format("Could not find htmlFiltering object for site: %s", siteKey));
logger.warn("Could not find htmlFiltering object for site: {}", siteKey);
}
}

Expand Down Expand Up @@ -121,7 +120,7 @@ public JSONObject getMergedJSONPolicy(String... siteKeys) {

for (String key : siteKeys) {
if (configExists(key)) {
mergeJsonObject(mergedPolicy, configs.get(siteKeyToPid.get(key)));
mergeJSONObject(mergedPolicy, configs.get(siteKeyToPid.get(key)));
}
}

Expand Down Expand Up @@ -150,27 +149,35 @@ public boolean htmlSanitizerDryRun(String siteKey) {
return f.has("htmlSanitizerDryRun") && f.getBoolean("htmlSanitizerDryRun");
}

private void mergeJsonObject(JSONObject target, JSONObject source) {
private void mergeJSONObject(JSONObject target, JSONObject source) {
for (String key : source.keySet()) {
if (source.get(key) instanceof JSONObject) {
if (target.has(key) && target.get(key) instanceof JSONObject) {
mergeJsonObject(target.getJSONObject(key), source.getJSONObject(key));
} else {
target.put(key, new JSONObject(source.getJSONObject(key).toString()));
}
mergeSubJSONObject(target, source, key);
} else if (source.get(key) instanceof JSONArray) {
if (target.has(key) && target.get(key) instanceof JSONArray) {
JSONArray targetArray = target.getJSONArray(key);
JSONArray sourceArray = source.getJSONArray(key);
for (int i = 0; i < sourceArray.length(); i++) {
targetArray.put(sourceArray.get(i));
}
} else {
target.put(key, new JSONArray(source.getJSONArray(key).toString()));
}
mergeJSONArray(target, source, key);
} else {
target.put(key, source.get(key));
}
}
}

private static void mergeJSONArray(JSONObject target, JSONObject source, String key) {
if (target.has(key) && target.get(key) instanceof JSONArray) {
JSONArray targetArray = target.getJSONArray(key);
JSONArray sourceArray = source.getJSONArray(key);
for (int i = 0; i < sourceArray.length(); i++) {
targetArray.put(sourceArray.get(i));
}
} else {
target.put(key, new JSONArray(source.getJSONArray(key).toString()));
}
}

private void mergeSubJSONObject(JSONObject target, JSONObject source, String key) {
if (target.has(key) && target.get(key) instanceof JSONObject) {
mergeJSONObject(target.getJSONObject(key), source.getJSONObject(key));
} else {
target.put(key, new JSONObject(source.getJSONObject(key).toString()));
}
}
}
Loading

0 comments on commit d4b2ddb

Please sign in to comment.