Checkstyle Security - SecureRandom, non-static nested classes & final clone() #802
Conversation
|
I don't understand the reasons behind the two first points and how it is related tos ecurity. Could you elaborate a little more? |
| import java.util.List; | ||
| import java.util.Map; | ||
|
|
||
| import static org.junit.Assert.*; |
There was a problem hiding this comment.
didn't you just comment on imports like this earlier? :-p
There was a problem hiding this comment.
I missed it. 1 - 1 ;-)
There was a problem hiding this comment.
Static imports being the key difference here :) We are just loading one class here only anyway, to make the VM super happy!
There was a problem hiding this comment.
In other words: star static method imports? Goooooood. star import on classes? Baaad. :)
|
@leleuj A bit of elaboration on the first two:
(Also in the event that the nested class is public and non-static, creating instances of it generally lead to perf issues, because you'd require an object of the enclosing class before you can get to the nested class).
|
|
Point 1 makes sense. For point 2, if I make the |
Conflicts: cas-server-core/src/test/java/org/jasig/cas/ticket/registry/DistributedTicketRegistryTests.java cas-server-support-jdbc/src/test/java/org/jasig/cas/ticket/registry/JpaTicketRegistryTests.java cas-server-support-jdbc/src/test/java/org/jasig/cas/ticket/registry/support/JpaLockingStrategyTests.java
|
You can't, and you shouldn't. What you would choose instead is either a copy constructor or a static factory (which how RegisteredService instances are created) |
|
Assuming folks are OK with the changeset, I plan to merge by tomorrow evening EDT. |
Checkstyle Security - SecureRandom, non-static nested classes & final clone()
Conflicts: cas-server-core/src/test/java/org/jasig/cas/ticket/registry/DistributedTicketRegistryTests.java
So this pull addresses checkstyle security issues (actually, there is no inherent checks in CS for these so I came up with custom rules). All 3 of the following categories pertain to "security". Nothing serious, more like hardening the code:
Review should be pretty easy on the eye as the changes are fairly minimal. In some areas (tests specifically) I had to dance around the code a bit to make sure the now-made-static inner classes still would function without the access to the parent's fields.