Skip to content

Commit

Permalink
CR fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
Krystian Panek authored and toniedzwiedz committed Jun 29, 2020
1 parent f290cf8 commit c73220d
Show file tree
Hide file tree
Showing 5 changed files with 42 additions and 137 deletions.
3 changes: 2 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,5 @@ atlassian-ide-plugin.xml
.classpath
.project
.settings/
.java-version
.java-version
.DS_Store
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,14 @@

import com.cognifide.aemrules.metadata.Metadata;
import com.cognifide.aemrules.tag.Tags;
import com.cognifide.aemrules.utils.MultiMap;
import com.cognifide.aemrules.version.AemVersion;
import org.sonar.check.Priority;
import org.sonar.check.Rule;
import org.sonar.plugins.html.node.TagNode;

import java.util.Collection;
import java.util.List;
import java.util.Map;
import java.util.regex.Pattern;

@Rule(
Expand All @@ -52,36 +53,35 @@ public class DefaultDisplayContextCheck extends AbstractHtlCheck {

private static final String VIOLATION_MESSAGE = "Explicitly using default display context, please remove display context from expression";

private static final MultiMap<String, String> TAG_ATTRIBUTE_MAPPING = MultiMap.builder(m -> {
m.add("form", "action");
m.add("blockquote", "cite");
m.add("del", "cite");
m.add("ins", "cite");
m.add("q", "cite");
m.add("object", "data");
m.add("button", "formaction");
m.add("input", "formaction");
m.add("a", "href");
m.add("area", "href");
m.add("link", "href");
m.add("base", "href");
m.add("html", "manifest");
m.add("video", "poster");
m.add("audio", "src");
m.add("embed", "src");
m.add("iframe", "src");
m.add("img", "src");
m.add("input", "src");
m.add("script", "src");
m.add("source", "src");
m.add("track", "src");
});
private static final Map<String, List<String>> TAG_ATTRIBUTE_MAPPING = Map.ofEntries(
Map.entry("a", List.of("href")),
Map.entry("area", List.of("href")),
Map.entry("audio", List.of("src")),
Map.entry("base", List.of("href")),
Map.entry("blockquote", List.of("cite")),
Map.entry("button", List.of("formaction")),
Map.entry("del", List.of("cite")),
Map.entry("embed", List.of("src")),
Map.entry("form", List.of("action")),
Map.entry("html", List.of("manifest")),
Map.entry("img", List.of("src")),
Map.entry("ins", List.of("cite")),
Map.entry("input", List.of("formaction", "src")),
Map.entry("iframe", List.of("src")),
Map.entry("link", List.of("href")),
Map.entry("q", List.of("cite")),
Map.entry("object", List.of("data")),
Map.entry("video", List.of("poster")),
Map.entry("script", List.of("src")),
Map.entry("source", List.of("src")),
Map.entry("track", List.of("src"))
);

@Override
public void startElement(TagNode node) {
String nodeName = node.getNodeName();
if (TAG_ATTRIBUTE_MAPPING.containsKey(nodeName)) {
Collection<String> supportedAttributes = TAG_ATTRIBUTE_MAPPING.getAll(nodeName);
Collection<String> supportedAttributes = TAG_ATTRIBUTE_MAPPING.get(nodeName);
node.getAttributes().stream()
.filter(attribute -> supportedAttributes.contains(attribute.getName()))
.filter(a -> CONTEXT_URI_DEFINITION.matcher(a.getValue()).find())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,11 @@

class ElementTokenizer extends AbstractTokenizer<List<Node>> {

private static final EndQNameMatcher endQNameMatcher = new EndQNameMatcher();
private static final EndQNameMatcher END_Q_NAME_MATCHER = new EndQNameMatcher();

private static final EndTokenMatcher endTokenMatcher = new EndTokenMatcher();
private static final EndTokenMatcher END_TOKEN_MATCHER = new EndTokenMatcher();

private static final EndUnquotedAttributeMatcher endUnquotedAttributeMatcher = new EndUnquotedAttributeMatcher();
private static final EndUnquotedAttributeMatcher END_UNQUOTED_ATTRIBUTE_MATCHER = new EndUnquotedAttributeMatcher();

public ElementTokenizer(String startToken, String endToken) {
super(startToken, endToken);
Expand Down Expand Up @@ -80,7 +80,7 @@ private static void handleBeforeAttributeValue(CodeReader codeReader, TagNode el
codeReader.pop();
attribute.setQuoteChar((char) ch);
} else {
codeReader.popTo(endUnquotedAttributeMatcher, sbValue);
codeReader.popTo(END_UNQUOTED_ATTRIBUTE_MATCHER, sbValue);
attribute.setValue(sbValue.toString().trim());
}
}
Expand All @@ -89,15 +89,15 @@ private static void handleBeforeAttributeValue(CodeReader codeReader, TagNode el
private static void handleBeforeAttributeName(CodeReader codeReader, TagNode element) {
Attribute attribute;
StringBuilder sbQName = new StringBuilder();
codeReader.popTo(endQNameMatcher, sbQName);
codeReader.popTo(END_Q_NAME_MATCHER, sbQName);
attribute = new Attribute(sbQName.toString().trim());
attribute.setLine(codeReader.getLinePosition() + element.getStartLinePosition() - 1);
element.getAttributes().add(attribute);
}

private static void handleBeforeNodeName(CodeReader codeReader, TagNode element) {
StringBuilder sbNodeName = new StringBuilder();
codeReader.popTo(endTokenMatcher, sbNodeName);
codeReader.popTo(END_TOKEN_MATCHER, sbNodeName);
element.setNodeName(sbNodeName.toString());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,24 +55,24 @@ public class ThreadSafeFieldCheck extends BaseTreeVisitor implements JavaFileSca

public static final String RULE_MESSAGE = "Usage of %s as a field is not thread safe.";

private static final Set<String> vulnerableClasses = Set.of(
private static final Set<String> VULNERABLE_CLASSES = Set.of(
// empty for now
);

private static final Set<String> vulnerableInterfaces = Set.of(
private static final Set<String> VULNERABLE_INTERFACES = Set.of(
"javax.servlet.Servlet",
"javax.servlet.Filter",
"org.osgi.service.event.EventHandler"
);

private static final Set<String> vulnerableAnnotations = Set.of(
private static final Set<String> VULNERABLE_ANNOTATIONS = Set.of(
"org.apache.felix.scr.annotations.Component",
"org.osgi.service.component.annotations.Component",
"org.apache.felix.scr.annotations.sling.SlingServlet", // this is possibly duplicative, but that shouldn't be a problem.
"org.apache.felix.scr.annotations.sling.SlingFilter" // this is possibly duplicative, but that shouldn't be a problem.
);

private static final Set<String> nonThreadSafeTypes = Set.of(
private static final Set<String> NON_THREAD_SAFE_TYPES = Set.of(
"org.apache.sling.api.resource.ResourceResolver",
"javax.jcr.Session",
"com.day.cq.wcm.api.PageManager",
Expand Down Expand Up @@ -116,7 +116,7 @@ private void checkMember(Tree member) {
if (isVariableField) {
VariableTree variableField = (VariableTree) member;
String name = variableField.type().symbolType().fullyQualifiedName();
if (nonThreadSafeTypes.contains(name)) {
if (NON_THREAD_SAFE_TYPES.contains(name)) {
context.reportIssue(this, member, String.format(RULE_MESSAGE, name));
}
}
Expand All @@ -126,7 +126,7 @@ private boolean hasAnnotation(ClassTree clazz) {
boolean hasAnnotation = false;
for (AnnotationTree annotationTree : clazz.modifiers().annotations()) {
String name = annotationTree.annotationType().symbolType().fullyQualifiedName();
hasAnnotation |= vulnerableAnnotations.contains(name);
hasAnnotation |= VULNERABLE_ANNOTATIONS.contains(name);
}
return hasAnnotation;
}
Expand All @@ -136,7 +136,7 @@ private boolean extendsVulnerableClass(ClassTree clazz) {
TypeTree type = clazz.superClass();
if (type != null) {
String name = type.symbolType().fullyQualifiedName();
extendsClass = vulnerableClasses.contains(name);
extendsClass = VULNERABLE_CLASSES.contains(name);
}
return extendsClass;
}
Expand All @@ -145,7 +145,7 @@ private boolean implementsVulnerableInterface(ClassTree clazz) {
boolean implementsInterface = false;
for (TypeTree typeTree : clazz.superInterfaces()) {
String name = typeTree.symbolType().fullyQualifiedName();
implementsInterface |= vulnerableInterfaces.contains(name);
implementsInterface |= VULNERABLE_INTERFACES.contains(name);
}
return implementsInterface;
}
Expand Down
96 changes: 0 additions & 96 deletions src/main/java/com/cognifide/aemrules/utils/MultiMap.java

This file was deleted.

0 comments on commit c73220d

Please sign in to comment.