MCHECKSTYLE-366 Upgrade checkstyle to a more recent version #13
Conversation
and require Java 8 as Checkstyle 7+ requires Java 8
|
@rnveach I have addressed your comment, please take another look |
|
This is the CI job for this branch: |
|
Looks good, thanks. |
| @@ -20,8 +20,6 @@ under the License. | |||
| <!DOCTYPE module PUBLIC "-//Puppy Crawl//DTD Check Configuration 1.2//EN" "http://www.puppycrawl.com/dtds/configuration_1_2.dtd"> | |||
There was a problem hiding this comment.
it would be good if you can update all xmls to have new DOCTYPE that reference to secure website.
Example: https://github.com/checkstyle/checkstyle/blob/master/config/checkstyle_checks.xml#L2
There should be no Puppy Crawl, no http://.
it is part of security issue - checkstyle/checkstyle#6478 and checkstyle/checkstyle#6474
There was a problem hiding this comment.
This is a test resource
There was a problem hiding this comment.
@eolivelli , please address my comment, to avoid security issues in plugin.
There was a problem hiding this comment.
@romani this file is a test resource, it is an integration test.
It would be better to create a specific JIRA and track the work to do.
I will be very happy to fix the problem you are reporting or to help your provide a patch.
This PR has already been merged, we should start a new one
There was a problem hiding this comment.
@eolivelli The security issue is in the parser that is used to parse this file. It leaves any code that parses this file vulnerable to XXE via a MITM.
It doesn't matter that it's a "test resource", it still leaves the build vulnerable to this attack vector.
There was a problem hiding this comment.
Here is the new ticket to provide any patches for: https://issues.apache.org/jira/browse/MCHECKSTYLE-375
There was a problem hiding this comment.
Great. If anyone wants to pick it up....I can wait for it before cutting the 3.1.0 release.
I am currently waiting for another patch se the is no hurry.
There was a problem hiding this comment.
@eolivelli , please share link to patch that you are waiting
|
This pr has been merged, sorry, I did not close it |
MCHECKSTYLE-366 Upgrade checkstyle to a more recent version
and require Java 8 as Checkstyle 7+ requires Java 8
Fix several integration tests that didn't work with new versions of checkstyle