Skip to content

Commit 1179c3c

Browse files
rbrirbri
committed
code cleanup, pmd suggestions
1 parent 4fe7bf5 commit 1179c3c

File tree

8 files changed

+138
-136
lines changed

8 files changed

+138
-136
lines changed

‎pmd-ruleset.xml

Lines changed: 33 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -8,53 +8,70 @@
88
<description>HtmlUnit rules</description>
99
<exclude-pattern>.*/src/test/resources/.*</exclude-pattern>
1010

11-
<rule ref="category/java/bestpractices.xml" />
11+
<rule ref="category/java/codestyle.xml/UnnecessaryFullyQualifiedName">
12+
<properties>
13+
<property name="reportStaticMethods" value="false" />
14+
<property name="reportStaticFields" value="false" />
15+
</properties>
16+
</rule>
17+
18+
<!-- i think there is no need to guard error log statements -->
19+
<rule ref="category/java/bestpractices.xml/GuardLogStatement">
20+
<properties>
21+
<property name="logLevels" value="trace,debug,info,warn,log,finest,finer,fine,info,warning" />
22+
</properties>
23+
</rule>
24+
25+
<rule ref="category/java/bestpractices.xml">
26+
<exclude name="AccessorMethodGeneration"/>
27+
</rule>
1228

1329
<rule ref="category/java/codestyle.xml">
1430
<exclude name="AtLeastOneConstructor"/>
15-
<exclude name="AvoidFinalLocalVariable"/>
16-
<exclude name="AtLeastOneConstructor"/>
17-
<exclude name="CallSuperInConstructor"/>
1831
<exclude name="ClassNamingConventions"/>
1932
<exclude name="CommentDefaultAccessModifier"/>
20-
<exclude name="DefaultPackage"/>
2133
<exclude name="FieldNamingConventions"/>
34+
<exclude name="IdenticalCatchBranches"/>
2235
<exclude name="LongVariable"/>
2336
<exclude name="MethodNamingConventions"/>
2437
<exclude name="OnlyOneReturn"/>
38+
<exclude name="ShortClassName"/>
39+
<exclude name="ShortMethodName"/>
2540
<exclude name="ShortVariable"/>
26-
<exclude name="TooManyStaticImports"/>
27-
<exclude name="UnnecessaryLocalBeforeReturn"/>
28-
<exclude name="UnnecessaryConstructor"/>
29-
<exclude name="UnnecessaryFullyQualifiedName"/>
30-
<exclude name="UselessParentheses"/>
41+
<exclude name="UnnecessaryBoxing"/>
3142
</rule>
3243

3344
<rule ref="category/java/design.xml">
45+
<exclude name="AvoidDeeplyNestedIfStmts"/>
46+
<exclude name="CognitiveComplexity"/>
47+
<exclude name="CollapsibleIfStatements"/>
48+
<exclude name="CouplingBetweenObjects"/>
3449
<exclude name="CyclomaticComplexity"/>
35-
<exclude name="ExcessiveClassLength"/>
50+
<exclude name="ExcessiveImports"/>
51+
<exclude name="ExcessivePublicCount"/>
52+
<exclude name="GodClass"/>
3653
<exclude name="LawOfDemeter"/>
54+
<exclude name="NcssCount"/>
3755
<exclude name="NPathComplexity"/>
56+
<exclude name="SimplifyBooleanReturns"/>
57+
<exclude name="TooManyMethods"/>
3858
</rule>
3959

4060
<rule ref="category/java/documentation.xml">
4161
<exclude name="CommentSize"/>
4262
<exclude name="CommentRequired"/>
43-
<exclude name="UncommentedEmptyConstructor"/>
4463
</rule>
4564

4665
<rule ref="category/java/errorprone.xml">
47-
<exclude name="AvoidDuplicateLiterals"/>
4866
<exclude name="AvoidLiteralsInIfCondition"/>
49-
<exclude name="BeanMembersShouldSerialize"/>
50-
<exclude name="DataflowAnomalyAnalysis"/>
5167
<exclude name="MissingSerialVersionUID"/>
5268
</rule>
5369

5470
<rule ref="category/java/multithreading.xml" />
5571

5672
<rule ref="category/java/performance.xml">
57-
<exclude name="AvoidUsingShortType"/>
73+
<!-- todo reenable this and check -->
74+
<exclude name="AvoidInstantiatingObjectsInLoops"/>
5875
</rule>
5976

6077
<rule ref="category/java/security.xml" />

‎src/main/java/org/htmlunit/csp/FetchDirectiveKind.java

Lines changed: 23 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,29 @@ public enum FetchDirectiveKind {
6666
/** WorkerSrc("worker-src"). */
6767
WorkerSrc("worker-src");
6868

69+
// https://w3c.github.io/webappsec-csp/#directive-fallback-list
70+
// Note the oddity that worker-src falls back to child-src then script-src
71+
// then directive-src, but frame-src falls back to child-src then directly default-src
72+
// Also note that `script-src` falls back to `default-src` for "unsafe-eval", but this
73+
// is done manually in prose rather than in this table
74+
// (in https://w3c.github.io/webappsec-csp/#can-compile-strings )
75+
// It is included here only for completeness
76+
private static final FetchDirectiveKind[] ScriptSrcFallback = {ScriptSrc, DefaultSrc};
77+
private static final FetchDirectiveKind[] ScriptSrcElemFallback = {ScriptSrcElem, ScriptSrc, DefaultSrc};
78+
private static final FetchDirectiveKind[] ScriptSrcAttrFallback = {ScriptSrcAttr, ScriptSrc, DefaultSrc};
79+
private static final FetchDirectiveKind[] StyleSrcFallback = {StyleSrc, DefaultSrc};
80+
private static final FetchDirectiveKind[] StyleSrcElemFallback = {StyleSrcElem, StyleSrc, DefaultSrc};
81+
private static final FetchDirectiveKind[] StyleSrcAttrFallback = {StyleSrcAttr, StyleSrc, DefaultSrc};
82+
private static final FetchDirectiveKind[] WorkerSrcFallback = {WorkerSrc, ChildSrc, ScriptSrc, DefaultSrc};
83+
private static final FetchDirectiveKind[] ConnectSrcFallback = {ConnectSrc, DefaultSrc};
84+
private static final FetchDirectiveKind[] ManifestSrcFallback = {ManifestSrc, DefaultSrc};
85+
private static final FetchDirectiveKind[] PrefetchSrcFallback = {PrefetchSrc, DefaultSrc};
86+
private static final FetchDirectiveKind[] ObjectSrcFallback = {ObjectSrc, DefaultSrc};
87+
private static final FetchDirectiveKind[] FrameSrcFallback = {FrameSrc, ChildSrc, DefaultSrc };
88+
private static final FetchDirectiveKind[] MediaSrcFallback = {MediaSrc, DefaultSrc };
89+
private static final FetchDirectiveKind[] FontSrcFallback = {FontSrc, DefaultSrc };
90+
private static final FetchDirectiveKind[] ImgSrcFallback = {ImgSrc, DefaultSrc };
91+
6992
private final String repr_;
7093

7194
FetchDirectiveKind(final String repr) {
@@ -118,44 +141,6 @@ public static FetchDirectiveKind fromString(final String name) {
118141
}
119142
}
120143

121-
// https://w3c.github.io/webappsec-csp/#directive-fallback-list
122-
// Note the oddity that worker-src falls back to child-src then script-src
123-
// then directive-src, but frame-src falls back to child-src then directly default-src
124-
// Also note that `script-src` falls back to `default-src` for "unsafe-eval", but this
125-
// is done manually in prose rather than in this table
126-
// (in https://w3c.github.io/webappsec-csp/#can-compile-strings )
127-
// It is included here only for completeness
128-
private static final FetchDirectiveKind[] ScriptSrcFallback
129-
= new FetchDirectiveKind[] {ScriptSrc, DefaultSrc};
130-
private static final FetchDirectiveKind[] ScriptSrcElemFallback
131-
= new FetchDirectiveKind[] {ScriptSrcElem, ScriptSrc, DefaultSrc};
132-
private static final FetchDirectiveKind[] ScriptSrcAttrFallback
133-
= new FetchDirectiveKind[] {ScriptSrcAttr, ScriptSrc, DefaultSrc};
134-
private static final FetchDirectiveKind[] StyleSrcFallback
135-
= new FetchDirectiveKind[] {StyleSrc, DefaultSrc};
136-
private static final FetchDirectiveKind[] StyleSrcElemFallback
137-
= new FetchDirectiveKind[] {StyleSrcElem, StyleSrc, DefaultSrc};
138-
private static final FetchDirectiveKind[] StyleSrcAttrFallback
139-
= new FetchDirectiveKind[] {StyleSrcAttr, StyleSrc, DefaultSrc};
140-
private static final FetchDirectiveKind[] WorkerSrcFallback
141-
= new FetchDirectiveKind[] {WorkerSrc, ChildSrc, ScriptSrc, DefaultSrc};
142-
private static final FetchDirectiveKind[] ConnectSrcFallback
143-
= new FetchDirectiveKind[] {ConnectSrc, DefaultSrc};
144-
private static final FetchDirectiveKind[] ManifestSrcFallback
145-
= new FetchDirectiveKind[] {ManifestSrc, DefaultSrc};
146-
private static final FetchDirectiveKind[] PrefetchSrcFallback
147-
= new FetchDirectiveKind[] {PrefetchSrc, DefaultSrc};
148-
private static final FetchDirectiveKind[] ObjectSrcFallback
149-
= new FetchDirectiveKind[] {ObjectSrc, DefaultSrc};
150-
private static final FetchDirectiveKind[] FrameSrcFallback
151-
= new FetchDirectiveKind[] {FrameSrc, ChildSrc, DefaultSrc };
152-
private static final FetchDirectiveKind[] MediaSrcFallback
153-
= new FetchDirectiveKind[] {MediaSrc, DefaultSrc };
154-
private static final FetchDirectiveKind[] FontSrcFallback
155-
= new FetchDirectiveKind[] {FontSrc, DefaultSrc };
156-
private static final FetchDirectiveKind[] ImgSrcFallback
157-
= new FetchDirectiveKind[] {ImgSrc, DefaultSrc };
158-
159144
static FetchDirectiveKind[] getFetchDirectiveFallbackList(final FetchDirectiveKind directive) {
160145
switch (directive) {
161146
case ScriptSrc:

‎src/main/java/org/htmlunit/csp/Policy.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ public final class Policy {
5858
// - Duplicate values
5959
// - Unrecognized values
6060

61-
private List<NamedDirective> directives_ = new ArrayList<>();
61+
private final List<NamedDirective> directives_ = new ArrayList<>();
6262

6363
private SourceExpressionDirective baseUri_;
6464
private boolean blockAllMixedContent_;

‎src/main/java/org/htmlunit/csp/directive/HostSourceDirective.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,8 @@
2828
public abstract class HostSourceDirective extends Directive {
2929
private static final String NONE_SRC = "'none'";
3030
private static final String SELF_SRC = "'self'";
31-
private List<Scheme> schemes_ = new ArrayList<>();
32-
private List<Host> hosts_ = new ArrayList<>();
31+
private final List<Scheme> schemes_ = new ArrayList<>();
32+
private final List<Host> hosts_ = new ArrayList<>();
3333
private boolean star_;
3434
private boolean self_;
3535

‎src/main/java/org/htmlunit/csp/directive/PluginTypesDirective.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@
2424
import org.htmlunit.csp.value.MediaType;
2525

2626
public class PluginTypesDirective extends Directive {
27-
private List<MediaType> mediaTypes_ = new ArrayList<>();
27+
private final List<MediaType> mediaTypes_ = new ArrayList<>();
2828

2929
public PluginTypesDirective(final List<String> values, final DirectiveErrorConsumer errors) {
3030
super(values);

‎src/main/java/org/htmlunit/csp/directive/ReportUriDirective.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
import org.htmlunit.csp.Policy;
2323

2424
public class ReportUriDirective extends Directive {
25-
private List<String> uris_ = new ArrayList<>();
25+
private final List<String> uris_ = new ArrayList<>();
2626

2727
public ReportUriDirective(final List<String> values, final DirectiveErrorConsumer errors) {
2828
super(values);

‎src/main/java/org/htmlunit/csp/directive/SandboxDirective.java

Lines changed: 55 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -22,19 +22,19 @@
2222

2323
public class SandboxDirective extends Directive {
2424
private static final String ALLOW_DOWNLOADS = "allow-downloads";
25-
private boolean allowDownloads_ = false;
26-
private boolean allowForms_ = false;
27-
private boolean allowModals_ = false;
28-
private boolean allowOrientationLock_ = false;
29-
private boolean allowPointerLock_ = false;
30-
private boolean allowPopups_ = false;
31-
private boolean allowPopupsToEscapeSandbox_ = false;
32-
private boolean allowPresentation_ = false;
33-
private boolean allowSameOrigin_ = false;
34-
private boolean allowScripts_ = false;
35-
private boolean allowStorageAccessByUserActivation_ = false;
36-
private boolean allowTopNavigation_ = false;
37-
private boolean allowTopNavigationByUserActivation_ = false;
25+
private boolean allowDownloads_;
26+
private boolean allowForms_;
27+
private boolean allowModals_;
28+
private boolean allowOrientationLock_;
29+
private boolean allowPointerLock_;
30+
private boolean allowPopups_;
31+
private boolean allowPopupsToEscapeSandbox_;
32+
private boolean allowPresentation_;
33+
private boolean allowSameOrigin_;
34+
private boolean allowScripts_;
35+
private boolean allowStorageAccessByUserActivation_;
36+
private boolean allowTopNavigation_;
37+
private boolean allowTopNavigationByUserActivation_;
3838

3939
public SandboxDirective(final List<String> values, final DirectiveErrorConsumer errors) {
4040
super(values);
@@ -46,111 +46,111 @@ public SandboxDirective(final List<String> values, final DirectiveErrorConsumer
4646
final String lowcaseToken = token.toLowerCase(Locale.ROOT);
4747
switch (lowcaseToken) {
4848
case ALLOW_DOWNLOADS:
49-
if (!allowDownloads_) {
50-
allowDownloads_ = true;
49+
if (allowDownloads_) {
50+
errors.add(Policy.Severity.Warning, "Duplicate sandbox keyword allow-downloads", index);
5151
}
5252
else {
53-
errors.add(Policy.Severity.Warning, "Duplicate sandbox keyword allow-downloads", index);
53+
allowDownloads_ = true;
5454
}
5555
break;
5656
case "allow-forms":
57-
if (!allowForms_) {
58-
allowForms_ = true;
57+
if (allowForms_) {
58+
errors.add(Policy.Severity.Warning, "Duplicate sandbox keyword allow-forms", index);
5959
}
6060
else {
61-
errors.add(Policy.Severity.Warning, "Duplicate sandbox keyword allow-forms", index);
61+
allowForms_ = true;
6262
}
6363
break;
6464
case "allow-modals":
65-
if (!allowModals_) {
66-
allowModals_ = true;
65+
if (allowModals_) {
66+
errors.add(Policy.Severity.Warning, "Duplicate sandbox keyword allow-modals", index);
6767
}
6868
else {
69-
errors.add(Policy.Severity.Warning, "Duplicate sandbox keyword allow-modals", index);
69+
allowModals_ = true;
7070
}
7171
break;
7272
case "allow-orientation-lock":
73-
if (!allowOrientationLock_) {
74-
allowOrientationLock_ = true;
73+
if (allowOrientationLock_) {
74+
errors.add(Policy.Severity.Warning, "Duplicate sandbox keyword allow-orientation-lock", index);
7575
}
7676
else {
77-
errors.add(Policy.Severity.Warning, "Duplicate sandbox keyword allow-orientation-lock", index);
77+
allowOrientationLock_ = true;
7878
}
7979
break;
8080
case "allow-pointer-lock":
81-
if (!allowPointerLock_) {
82-
allowPointerLock_ = true;
81+
if (allowPointerLock_) {
82+
errors.add(Policy.Severity.Warning, "Duplicate sandbox keyword allow-pointer-lock", index);
8383
}
8484
else {
85-
errors.add(Policy.Severity.Warning, "Duplicate sandbox keyword allow-pointer-lock", index);
85+
allowPointerLock_ = true;
8686
}
8787
break;
8888
case "allow-popups":
89-
if (!allowPopups_) {
90-
allowPopups_ = true;
89+
if (allowPopups_) {
90+
errors.add(Policy.Severity.Warning, "Duplicate sandbox keyword allow-popups", index);
9191
}
9292
else {
93-
errors.add(Policy.Severity.Warning, "Duplicate sandbox keyword allow-popups", index);
93+
allowPopups_ = true;
9494
}
9595
break;
9696
case "allow-popups-to-escape-sandbox":
97-
if (!allowPopupsToEscapeSandbox_) {
98-
allowPopupsToEscapeSandbox_ = true;
99-
}
100-
else {
97+
if (allowPopupsToEscapeSandbox_) {
10198
errors.add(Policy.Severity.Warning,
10299
"Duplicate sandbox keyword allow-popups-to-escape-sandbox", index);
103100
}
101+
else {
102+
allowPopupsToEscapeSandbox_ = true;
103+
}
104104
break;
105105
case "allow-presentation":
106-
if (!allowPresentation_) {
107-
allowPresentation_ = true;
106+
if (allowPresentation_) {
107+
errors.add(Policy.Severity.Warning, "Duplicate sandbox keyword allow-presentation", index);
108108
}
109109
else {
110-
errors.add(Policy.Severity.Warning, "Duplicate sandbox keyword allow-presentation", index);
110+
allowPresentation_ = true;
111111
}
112112
break;
113113
case "allow-same-origin":
114-
if (!allowSameOrigin_) {
115-
allowSameOrigin_ = true;
114+
if (allowSameOrigin_) {
115+
errors.add(Policy.Severity.Warning, "Duplicate sandbox keyword allow-same-origin", index);
116116
}
117117
else {
118-
errors.add(Policy.Severity.Warning, "Duplicate sandbox keyword allow-same-origin", index);
118+
allowSameOrigin_ = true;
119119
}
120120
break;
121121
case "allow-scripts":
122-
if (!allowScripts_) {
123-
allowScripts_ = true;
122+
if (allowScripts_) {
123+
errors.add(Policy.Severity.Warning, "Duplicate sandbox keyword allow-scripts", index);
124124
}
125125
else {
126-
errors.add(Policy.Severity.Warning, "Duplicate sandbox keyword allow-scripts", index);
126+
allowScripts_ = true;
127127
}
128128
break;
129129
case "allow-storage-access-by-user-activation":
130-
if (!allowStorageAccessByUserActivation_) {
131-
allowStorageAccessByUserActivation_ = true;
132-
}
133-
else {
130+
if (allowStorageAccessByUserActivation_) {
134131
errors.add(Policy.Severity.Warning,
135132
"Duplicate sandbox keyword allow-storage-access-by-user-activation", index);
136133
}
134+
else {
135+
allowStorageAccessByUserActivation_ = true;
136+
}
137137
break;
138138
case "allow-top-navigation":
139-
if (!allowTopNavigation_) {
140-
allowTopNavigation_ = true;
139+
if (allowTopNavigation_) {
140+
errors.add(Policy.Severity.Warning, "Duplicate sandbox keyword allow-top-navigation", index);
141141
}
142142
else {
143-
errors.add(Policy.Severity.Warning, "Duplicate sandbox keyword allow-top-navigation", index);
143+
allowTopNavigation_ = true;
144144
}
145145
break;
146146
case "allow-top-navigation-by-user-activation":
147-
if (!allowTopNavigationByUserActivation_) {
148-
allowTopNavigationByUserActivation_ = true;
149-
}
150-
else {
147+
if (allowTopNavigationByUserActivation_) {
151148
errors.add(Policy.Severity.Warning,
152149
"Duplicate sandbox keyword allow-top-navigation-by-user-activation", index);
153150
}
151+
else {
152+
allowTopNavigationByUserActivation_ = true;
153+
}
154154
break;
155155
default:
156156
if (token.startsWith("'")) {

0 commit comments

Comments
 (0)