Skip to content

Commit

Permalink
add checks for inline javascript
Browse files Browse the repository at this point in the history
Detects usage of inline event handlers like onclick, usage of script
elements (except for json type), usage of checkUrl without
checkDependsOn

Same as
jenkins-infra/repository-permissions-updater#4049
but for maven builds
  • Loading branch information
mawinter69 committed Oct 30, 2024
1 parent 94aba5e commit f45a7c8
Showing 1 changed file with 51 additions and 2 deletions.
53 changes: 51 additions & 2 deletions src/main/java/org/jvnet/hudson/test/JellyTestSuiteBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,11 @@
import java.io.File;
import java.io.IOException;
import java.net.URL;
import java.util.ArrayList;
import java.util.Enumeration;
import java.util.HashMap;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.concurrent.Callable;
import java.util.jar.JarEntry;
Expand All @@ -36,7 +39,11 @@
import junit.framework.TestResult;
import junit.framework.TestSuite;
import org.apache.commons.io.FileUtils;
import org.dom4j.Attribute;
import org.dom4j.Document;
import org.dom4j.Element;
import org.dom4j.Namespace;
import org.dom4j.Node;
import org.dom4j.ProcessingInstruction;
import org.dom4j.io.SAXReader;
import org.jvnet.hudson.test.junit.GroupedTest;
Expand Down Expand Up @@ -88,6 +95,7 @@ private static class JellyCheck extends TestCase {
private final URL jelly;
private final JellyClassLoaderTearOff jct;
private final boolean requirePI;
private List<String> errors = new ArrayList<>();

JellyCheck(URL jelly, String name, JellyClassLoaderTearOff jct, boolean requirePI) {
super(name);
Expand All @@ -103,11 +111,52 @@ protected void runTest() throws Exception {
if (requirePI) {
ProcessingInstruction pi = dom.processingInstruction("jelly");
if (pi == null || !pi.getText().contains("escape-by-default")) {
throw new AssertionError("<?jelly escape-by-default='true'?> is missing in "+jelly);
errors.add("<?jelly escape-by-default='true'?> is missing in "+jelly);
}

}
// TODO: what else can we check statically? use of taglibs?
checkScriptElement(dom);
checkJavaScriptAttributes(dom);
if (!errors.isEmpty()) {
String message = String.join("\n", errors);
throw new AssertionError(message);
}
}

private void checkJavaScriptAttributes(Document dom) {
List<Node> allNodes = dom.selectNodes("//*");
allNodes.forEach(n -> {
Element element = (Element) n;
Attribute onclick = element.attribute("onclick");
Attribute checkUrl = element.attribute("checkUrl");
Attribute checkDependsOn = element.attribute("checkDependsOn");
if (checkUrl != null && checkDependsOn == null) {
errors.add("Usage of 'checkUrl' without 'checkDependsOn' in "+jelly);
}
if (onclick != null && element.getNamespace() != Namespace.NO_NAMESPACE) {
errors.add("Usage of 'onclick' from a taglib in "+jelly);
}
List<Attribute> attributes = element.attributes();
if (element.getNamespace() == Namespace.NO_NAMESPACE && !attributes.isEmpty()) {
attributes.forEach(a -> {
if (a.getName().startsWith("on")) {
errors.add("Usage of inline event handler '" + a.getName() + "' in "+jelly);
}
});
}
});
}

private void checkScriptElement(Document dom) {
List<Node> scriptTags = dom.selectNodes("//script");
scriptTags.forEach( n -> {
Element element = (Element) n;
String typeAttribute = element.attributeValue("type");
if (element.attributeValue("src") == null && (typeAttribute == null ||
!"application/json".equals(typeAttribute.toLowerCase(Locale.US)))) {
errors.add("inline <script> element in "+jelly);
}
});
}

private boolean isConfigJelly() {
Expand Down

0 comments on commit f45a7c8

Please sign in to comment.