Skip to content

Commit 25e81b8

Browse files
authored
1 parent 26a233c commit 25e81b8

File tree

7 files changed

+274
-0
lines changed

7 files changed

+274
-0
lines changed
Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
package io.codemodder.codemods;
2+
3+
import static io.codemodder.ast.ASTTransforms.addImportIfMissing;
4+
import static io.codemodder.ast.ASTTransforms.removeImportIfUnused;
5+
6+
import com.github.javaparser.ast.CompilationUnit;
7+
import com.github.javaparser.ast.body.ClassOrInterfaceDeclaration;
8+
import com.github.javaparser.ast.body.MethodDeclaration;
9+
import com.github.javaparser.ast.expr.AnnotationExpr;
10+
import com.github.javaparser.ast.expr.MarkerAnnotationExpr;
11+
import io.codemodder.*;
12+
import io.codemodder.providers.sonar.ProvidedSonarScan;
13+
import io.codemodder.providers.sonar.RuleIssues;
14+
import io.codemodder.providers.sonar.SonarPluginJavaParserChanger;
15+
import io.codemodder.providers.sonar.api.Issue;
16+
import java.util.List;
17+
import java.util.Optional;
18+
import javax.inject.Inject;
19+
20+
/**
21+
* A codemod to replace `@Controller` with `@RestController` and remove `@ResponseBody` annotations
22+
*/
23+
@Codemod(
24+
id = "sonar:java/simplify-rest-controller-annotations-s6833",
25+
reviewGuidance = ReviewGuidance.MERGE_WITHOUT_REVIEW,
26+
executionPriority = CodemodExecutionPriority.HIGH)
27+
public final class SimplifyRestControllerAnnotationsCodemod
28+
extends SonarPluginJavaParserChanger<ClassOrInterfaceDeclaration> {
29+
30+
@Inject
31+
public SimplifyRestControllerAnnotationsCodemod(
32+
@ProvidedSonarScan(ruleId = "java:S6833") final RuleIssues issues) {
33+
super(issues, ClassOrInterfaceDeclaration.class, RegionNodeMatcher.MATCHES_START);
34+
}
35+
36+
@Override
37+
public boolean onIssueFound(
38+
final CodemodInvocationContext context,
39+
final CompilationUnit cu,
40+
final ClassOrInterfaceDeclaration classOrInterfaceDeclaration,
41+
final Issue issue) {
42+
43+
final Optional<AnnotationExpr> controllerAnnotationOptional =
44+
classOrInterfaceDeclaration.getAnnotationByName("Controller");
45+
46+
if (controllerAnnotationOptional.isEmpty()) {
47+
return false;
48+
}
49+
50+
replaceControllerToRestControllerAnnotation(cu, controllerAnnotationOptional.get());
51+
52+
final Optional<AnnotationExpr> responseBodyClassAnnotationOptional =
53+
classOrInterfaceDeclaration.getAnnotationByName("ResponseBody");
54+
55+
responseBodyClassAnnotationOptional.ifPresent(AnnotationExpr::remove);
56+
57+
removeResponseBodyAnnotationFromClassMethods(classOrInterfaceDeclaration);
58+
59+
removeImportIfUnused(cu, "org.springframework.web.bind.annotation.ResponseBody");
60+
61+
return true;
62+
}
63+
64+
private void replaceControllerToRestControllerAnnotation(
65+
final CompilationUnit cu, final AnnotationExpr controllerAnnotation) {
66+
final AnnotationExpr restControllerAnnotation = new MarkerAnnotationExpr("RestController");
67+
controllerAnnotation.replace(restControllerAnnotation);
68+
removeImportIfUnused(cu, "org.springframework.stereotype.Controller");
69+
addImportIfMissing(cu, "org.springframework.web.bind.annotation.RestController");
70+
}
71+
72+
private void removeResponseBodyAnnotationFromClassMethods(
73+
final ClassOrInterfaceDeclaration classOrInterfaceDeclaration) {
74+
final List<MethodDeclaration> methods = classOrInterfaceDeclaration.getMethods();
75+
if (methods != null && !methods.isEmpty()) {
76+
methods.forEach(
77+
method -> {
78+
final Optional<AnnotationExpr> responseBodyMethodAnnotationOptional =
79+
method.getAnnotationByName("ResponseBody");
80+
responseBodyMethodAnnotationOptional.ifPresent(AnnotationExpr::remove);
81+
});
82+
}
83+
}
84+
}
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
This change makes it harder for developers to make a mistake when writing REST controllers in Spring. By marking the top level type with `@RestController`, it is now assumed that all the methods within it will return a Java object representing the response body. Thus, there is no need to specify, for each method, the `@ResponseBody` annotation.
2+
3+
Our changes look something like this:
4+
5+
```diff
6+
- import org.springframework.stereotype.Controller;
7+
- import org.springframework.web.bind.annotation.ResponseBody;
8+
+ import org.springframework.web.bind.annotation.RestController;
9+
- @Controller
10+
+ @RestController
11+
public class AccountController {
12+
...
13+
- @ResponseBody
14+
public AccountDetails viewAccount() {
15+
...
16+
```
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"summary" : "Replace `@Controller` with `@RestController` and remove `@ResponseBody` annotations (Sonar).",
3+
"change" : "Replace `@Controller` with `@RestController` and remove `@ResponseBody` annotations.",
4+
"references" : [
5+
"https://rules.sonarsource.com/java/RSPEC-6833/"
6+
]
7+
}
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
package io.codemodder.codemods;
2+
3+
import io.codemodder.testutils.CodemodTestMixin;
4+
import io.codemodder.testutils.Metadata;
5+
6+
@Metadata(
7+
codemodType = SimplifyRestControllerAnnotationsCodemod.class,
8+
testResourceDir = "simplify-rest-controller-annotations-s6833",
9+
renameTestFile = "src/main/java/org/owasp/webgoat/container/service/SessionService.java",
10+
dependencies = {})
11+
final class SimplifyRestControllerAnnotationsCodemodTest implements CodemodTestMixin {}
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
/*
2+
* To change this license header, choose License Headers in Project Properties.
3+
* To change this template file, choose Tools | Templates
4+
* and open the template in the editor.
5+
*/
6+
7+
package org.owasp.webgoat.container.service;
8+
9+
import lombok.RequiredArgsConstructor;
10+
import org.owasp.webgoat.container.i18n.Messages;
11+
import org.owasp.webgoat.container.session.WebSession;
12+
import org.springframework.web.bind.annotation.RequestMapping;
13+
import org.springframework.web.bind.annotation.RestController;
14+
15+
@RestController
16+
@RequiredArgsConstructor
17+
public class SessionService {
18+
19+
private final WebSession webSession;
20+
private final RestartLessonService restartLessonService;
21+
private final Messages messages;
22+
23+
@RequestMapping(path = "/service/enable-security.mvc", produces = "application/json")
24+
public String applySecurity() {
25+
webSession.toggleSecurity();
26+
restartLessonService.restartLesson();
27+
28+
var msg = webSession.isSecurityEnabled() ? "security.enabled" : "security.disabled";
29+
return messages.getMessage(msg);
30+
}
31+
}
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
/*
2+
* To change this license header, choose License Headers in Project Properties.
3+
* To change this template file, choose Tools | Templates
4+
* and open the template in the editor.
5+
*/
6+
7+
package org.owasp.webgoat.container.service;
8+
9+
import lombok.RequiredArgsConstructor;
10+
import org.owasp.webgoat.container.i18n.Messages;
11+
import org.owasp.webgoat.container.session.WebSession;
12+
import org.springframework.stereotype.Controller;
13+
import org.springframework.web.bind.annotation.RequestMapping;
14+
import org.springframework.web.bind.annotation.ResponseBody;
15+
16+
@Controller
17+
@RequiredArgsConstructor
18+
public class SessionService {
19+
20+
private final WebSession webSession;
21+
private final RestartLessonService restartLessonService;
22+
private final Messages messages;
23+
24+
@RequestMapping(path = "/service/enable-security.mvc", produces = "application/json")
25+
@ResponseBody
26+
public String applySecurity() {
27+
webSession.toggleSecurity();
28+
restartLessonService.restartLesson();
29+
30+
var msg = webSession.isSecurityEnabled() ? "security.enabled" : "security.disabled";
31+
return messages.getMessage(msg);
32+
}
33+
}
Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
{
2+
"total": 1,
3+
"p": 1,
4+
"ps": 500,
5+
"paging": {
6+
"pageIndex": 1,
7+
"pageSize": 500,
8+
"total": 1
9+
},
10+
"effortTotal": 5,
11+
"debtTotal": 5,
12+
"issues": [
13+
{
14+
"key": "AYvtrj4zLCzGLicz7AqC",
15+
"rule": "java:S6833",
16+
"severity": "MAJOR",
17+
"component": "nahsra_WebGoat_10_23:src/main/java/org/owasp/webgoat/container/service/SessionService.java",
18+
"project": "nahsra_WebGoat_10_23",
19+
"line": 16,
20+
"hash": "dee1a9dfa91ea501126d7a4ecdad8cd0",
21+
"textRange": {
22+
"startLine": 16,
23+
"endLine": 16,
24+
"startOffset": 0,
25+
"endOffset": 11
26+
},
27+
"flows": [
28+
{
29+
"locations": [
30+
{
31+
"component": "nahsra_WebGoat_10_23:src/main/java/org/owasp/webgoat/container/service/SessionService.java",
32+
"textRange": {
33+
"startLine": 25,
34+
"endLine": 25,
35+
"startOffset": 2,
36+
"endOffset": 15
37+
},
38+
"msg": "Remove this \"@ResponseBody\" annotation."
39+
}
40+
]
41+
}
42+
],
43+
"status": "OPEN",
44+
"message": "Replace the \"@Controller\" annotation by \"@RestController\" and remove all \"@ResponseBody\" annotations.",
45+
"effort": "5min",
46+
"debt": "5min",
47+
"tags": [
48+
"spring"
49+
],
50+
"creationDate": "2022-04-09T14:56:12+0200",
51+
"updateDate": "2023-11-20T17:58:54+0100",
52+
"type": "CODE_SMELL",
53+
"organization": "nahsra",
54+
"cleanCodeAttribute": "CONVENTIONAL",
55+
"cleanCodeAttributeCategory": "CONSISTENT",
56+
"impacts": [
57+
{
58+
"softwareQuality": "MAINTAINABILITY",
59+
"severity": "MEDIUM"
60+
}
61+
]
62+
}
63+
],
64+
"components": [
65+
{
66+
"organization": "nahsra",
67+
"key": "nahsra_WebGoat_10_23",
68+
"uuid": "AYvtqxxxzY4Rr5NHn4Om",
69+
"enabled": true,
70+
"qualifier": "TRK",
71+
"name": "WebGoat_10_23",
72+
"longName": "WebGoat_10_23"
73+
},
74+
{
75+
"organization": "nahsra",
76+
"key": "nahsra_WebGoat_10_23:src/main/java/org/owasp/webgoat/container/service/SessionService.java",
77+
"uuid": "AYvtrjqJLCzGLicz7Abj",
78+
"enabled": true,
79+
"qualifier": "FIL",
80+
"name": "SessionService.java",
81+
"longName": "src/main/java/org/owasp/webgoat/container/service/SessionService.java",
82+
"path": "src/main/java/org/owasp/webgoat/container/service/SessionService.java"
83+
}
84+
],
85+
"organizations": [
86+
{
87+
"key": "nahsra",
88+
"name": "Arshan Dabirsiaghi"
89+
}
90+
],
91+
"facets": []
92+
}

0 commit comments

Comments
 (0)