Skip to content

Commit

Permalink
Add checker for static final buffers (#12)
Browse files Browse the repository at this point in the history
Add validation for static final byte buffers to make sure they are unreleasable and read-only as a follow up for netty/netty#11792. Also enabled support for checkstyle warning suppression using `SuppressWarnings` annotation.

Verified by installing it locally and running it against local Netty code. Example:

```
→ mvn validate
[INFO] Scanning for projects...
[INFO] ------------------------------------------------------------------------
[INFO] Detecting the operating system and CPU architecture
[INFO] ------------------------------------------------------------------------
[INFO] os.detected.name: osx
[INFO] os.detected.arch: x86_64
[INFO] os.detected.bitness: 64
[INFO] os.detected.version: 10.16
[INFO] os.detected.version.major: 10
[INFO] os.detected.version.minor: 16
[INFO] os.detected.classifier: osx-x86_64
[INFO]
[INFO] ---------------------< io.netty:netty-codec-http >----------------------
[INFO] Building Netty/Codec/HTTP 4.1.70.Final-SNAPSHOT
[INFO] --------------------------------[ jar ]---------------------------------
[INFO]
[INFO] --- maven-enforcer-plugin:1.4.1:enforce (enforce-maven) @ netty-codec-http ---
[INFO]
[INFO] --- maven-enforcer-plugin:1.4.1:enforce (enforce-tools) @ netty-codec-http ---
[INFO]
[INFO] --- maven-checkstyle-plugin:3.1.0:check (check-style) @ netty-codec-http ---
[INFO] Starting audit...
[ERROR] /Users/alex.chekunkov/Dev/github.com/alexc-db/netty/codec-http/src/main/java/io/netty/handler/codec/http/HttpObjectEncoder.java:53: static final buffer assignment should match pattern (Unpooled\s*\.)?unreleasableBuffer\(.*?\)\s*\.asReadOnly\(\) [StaticFinalBuffer]
[ERROR] /Users/alex.chekunkov/Dev/github.com/alexc-db/netty/codec-http/src/main/java/io/netty/handler/codec/http/HttpObjectEncoder.java:54: static final buffer assignment should match pattern (Unpooled\s*\.)?unreleasableBuffer\(.*?\)\s*\.asReadOnly\(\) [StaticFinalBuffer]
[ERROR] /Users/alex.chekunkov/Dev/github.com/alexc-db/netty/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocket00FrameEncoder.java:34: static final buffer assignment should match pattern (Unpooled\s*\.)?unreleasableBuffer\(.*?\)\s*\.asReadOnly\(\) [StaticFinalBuffer]
[ERROR] /Users/alex.chekunkov/Dev/github.com/alexc-db/netty/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocket00FrameEncoder.java:36: static final buffer assignment should match pattern (Unpooled\s*\.)?unreleasableBuffer\(.*?\)\s*\.asReadOnly\(\) [StaticFinalBuffer]
[ERROR] /Users/alex.chekunkov/Dev/github.com/alexc-db/netty/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocket00FrameEncoder.java:38: static final buffer assignment should match pattern (Unpooled\s*\.)?unreleasableBuffer\(.*?\)\s*\.asReadOnly\(\) [StaticFinalBuffer]
Audit done.
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  3.755 s
[INFO] Finished at: 2021-10-26T20:00:56-07:00
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-checkstyle-plugin:3.1.0:check (check-style) on project netty-codec-http: Failed during checkstyle execution: There are 5 errors reported by Checkstyle 8.29 with io/netty/checkstyle.xml ruleset. -> [Help 1]
[ERROR]
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR]
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoExecutionException
```

Findings from the check are fixed in netty/netty#11802.
  • Loading branch information
alexc-db committed Oct 29, 2021
1 parent 05f79e9 commit 52567de
Show file tree
Hide file tree
Showing 2 changed files with 84 additions and 1 deletion.
@@ -0,0 +1,79 @@
/*
* Copyright 2021 The Netty Project
*
* The Netty Project licenses this file to you under the Apache License,
* version 2.0 (the "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at:
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
* License for the specific language governing permissions and limitations
* under the License.
*/

package io.netty.build.checkstyle;

import com.puppycrawl.tools.checkstyle.api.AbstractCheck;
import com.puppycrawl.tools.checkstyle.api.DetailAST;
import com.puppycrawl.tools.checkstyle.api.FileContents;
import com.puppycrawl.tools.checkstyle.api.FullIdent;
import com.puppycrawl.tools.checkstyle.api.TokenTypes;

import java.util.regex.Pattern;

/**
* This check verifies that static final buffers are unreleasable and read-only.
*
* It aims to prevent corruption bugs like https://github.com/netty/netty/issues/11792
* from happening in the future.
*/
public class StaticFinalBufferCheck extends AbstractCheck {

// Pattern is not multiline because variable definition is flattened and trimmed before the match.
private static final Pattern pattern = Pattern.compile(
"(Unpooled\\s*\\.)?unreleasableBuffer\\(.*?\\)\\s*\\.asReadOnly\\(\\)");

@Override
public int[] getRequiredTokens() {
return new int[]{TokenTypes.VARIABLE_DEF};
}

@Override
public int[] getDefaultTokens() {
return getRequiredTokens();
}

@Override
public int[] getAcceptableTokens() {
return getRequiredTokens();
}

@Override
public void visitToken(DetailAST ast) {
DetailAST modifiersAST = ast.findFirstToken(TokenTypes.MODIFIERS);
boolean isStatic = modifiersAST.findFirstToken(TokenTypes.LITERAL_STATIC) != null;
boolean isFinal = modifiersAST.findFirstToken(TokenTypes.FINAL) != null;
FullIdent typeIdent = FullIdent.createFullIdentBelow(ast.findFirstToken(TokenTypes.TYPE));
if (!isStatic || !isFinal || !typeIdent.getText().endsWith("Buf")) {
return;
}
DetailAST assignAST = ast.findFirstToken(TokenTypes.ASSIGN);
DetailAST semiAST = ast.findFirstToken(TokenTypes.SEMI);
if (assignAST == null || semiAST == null) {
log(ast.getLineNo(), "Missing assignment for static final buffer");
return;
}
FileContents fc = getFileContents();
StringBuilder sb = new StringBuilder();
for (int i = assignAST.getLineNo(); i <= semiAST.getLineNo(); i++) {
// getLineNo returns 1-based line number, getLine expects 0-based.
sb.append(fc.getLine(i - 1).trim());
}
if (!pattern.matcher(sb.toString()).find()) {
log(ast.getLineNo(), "static final buffer assignment should match pattern " + pattern);
}
}
}
6 changes: 5 additions & 1 deletion common/src/main/resources/io/netty/checkstyle.xml
Expand Up @@ -55,7 +55,7 @@
<module name="RegexpSingleline">
<property name="format" value="\s+$"/>
<property name="message" value="trailing whitespace"/>
</module>
</module>
<!-- Prohibit consecutive empty lines (except the lines after package/import) -->
<module name="RegexpMultiline">
<property name="format" value="\n *(?!package )(?!import )[^\n]+\n{3,}"/>
Expand All @@ -75,8 +75,12 @@
<property name="max" value="120"/>
<property name="fileExtensions" value="java"/>
</module>
<module name="SuppressWarningsFilter" />

<module name="TreeWalker">
<module name="SuppressWarningsHolder" />
<!-- Make sure final static buffers are unreleasable and read-only. -->
<module name="io.netty.build.checkstyle.StaticFinalBufferCheck"/>
<module name="WhitespaceAfter"/>
<module name="WhitespaceAround">
<property name="tokens" value="ASSIGN, BAND, BAND_ASSIGN, BOR, BOR_ASSIGN, BSR, BSR_ASSIGN, BXOR, BXOR_ASSIGN, DIV, DIV_ASSIGN, EQUAL, GE, GT, LAND, LCURLY, LE, LITERAL_ASSERT, LITERAL_CATCH, LITERAL_DO, LITERAL_ELSE, LITERAL_FINALLY, LITERAL_FOR, LITERAL_IF, LITERAL_RETURN, LITERAL_SYNCHRONIZED, LITERAL_TRY, LITERAL_WHILE, LOR, LT, MINUS, MINUS_ASSIGN, MOD, MOD_ASSIGN, NOT_EQUAL, PLUS, PLUS_ASSIGN, RCURLY, SL, SLIST, SL_ASSIGN, SR, SR_ASSIGN, STAR, STAR_ASSIGN, TYPE_EXTENSION_AND"/>
Expand Down

0 comments on commit 52567de

Please sign in to comment.