Skip to content

Commit f4b5f06

Browse files
guillaume-dequennesonartech
guillaume-dequenne
authored and
sonartech
committed
SONARPY-3612 Rule S6863 Flask error handlers should set HTTP status code (#819)
GitOrigin-RevId: 747a09fb93c42aa56d835e8469ee17c0ae1a99ce
1 parent a5253aa commit f4b5f06

7 files changed

Lines changed: 538 additions & 0 deletions

File tree

‎python-checks/src/main/java/org/sonar/python/checks/FlaskErrorHandlerStatusCheck.java‎

Lines changed: 201 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,201 @@
1+
/*
2+
* SonarQube Python Plugin
3+
* Copyright (C) 2011-2025 SonarSource Sàrl
4+
* mailto:info AT sonarsource DOT com
5+
*
6+
* This program is free software; you can redistribute it and/or
7+
* modify it under the terms of the Sonar Source-Available License Version 1, as published by SonarSource SA.
8+
*
9+
* This program is distributed in the hope that it will be useful,
10+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
11+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
12+
* See the Sonar Source-Available License for more details.
13+
*
14+
* You should have received a copy of the Sonar Source-Available License
15+
* along with this program; if not, see https://sonarsource.com/license/ssal/
16+
*/
17+
package org.sonar.python.checks;
18+
19+
import java.util.ArrayList;
20+
import java.util.List;
21+
import java.util.Optional;
22+
import org.sonar.check.Rule;
23+
import org.sonar.plugins.python.api.PythonSubscriptionCheck;
24+
import org.sonar.plugins.python.api.SubscriptionContext;
25+
import org.sonar.plugins.python.api.symbols.v2.SymbolV2;
26+
import org.sonar.plugins.python.api.symbols.v2.UsageV2;
27+
import org.sonar.plugins.python.api.tree.BaseTreeVisitor;
28+
import org.sonar.plugins.python.api.tree.CallExpression;
29+
import org.sonar.plugins.python.api.tree.Decorator;
30+
import org.sonar.plugins.python.api.tree.Expression;
31+
import org.sonar.plugins.python.api.tree.FunctionDef;
32+
import org.sonar.plugins.python.api.tree.Name;
33+
import org.sonar.plugins.python.api.tree.QualifiedExpression;
34+
import org.sonar.plugins.python.api.tree.ReturnStatement;
35+
import org.sonar.plugins.python.api.tree.Tree;
36+
import org.sonar.plugins.python.api.tree.Tuple;
37+
import org.sonar.plugins.python.api.types.v2.matchers.TypeMatchers;
38+
import org.sonar.python.checks.utils.Expressions;
39+
import org.sonar.python.tree.TreeUtils;
40+
41+
@Rule(key = "S6863")
42+
public class FlaskErrorHandlerStatusCheck extends PythonSubscriptionCheck {
43+
44+
private static final String MESSAGE = "Specify an explicit HTTP status code for this error handler.";
45+
46+
private static final String FLASK_APP_ERRORHANDLER_FQN = "flask.app.Flask.errorhandler";
47+
private static final String BLUEPRINT_ERRORHANDLER_FQN = "flask.blueprints.Blueprint.errorhandler";
48+
private static final String BLUEPRINT_APP_ERRORHANDLER_FQN = "flask.blueprints.Blueprint.app_errorhandler";
49+
50+
private static final String JSONIFY_FQN = "flask.json.jsonify";
51+
private static final String RENDER_TEMPLATE_FQN = "flask.templating.render_template";
52+
private static final String RENDER_TEMPLATE_STRING_FQN = "flask.templating.render_template_string";
53+
private static final String MAKE_RESPONSE_FQN = "flask.helpers.make_response";
54+
private static final String RESPONSE_FQN = "flask.wrappers.Response";
55+
56+
@Override
57+
public void initialize(Context context) {
58+
context.registerSyntaxNodeConsumer(Tree.Kind.FUNCDEF, FlaskErrorHandlerStatusCheck::checkErrorHandler);
59+
}
60+
61+
private static void checkErrorHandler(SubscriptionContext ctx) {
62+
FunctionDef functionDef = (FunctionDef) ctx.syntaxNode();
63+
if (!isErrorHandlerFunction(functionDef, ctx)) {
64+
return;
65+
}
66+
checkFunctionReturns(ctx, functionDef);
67+
}
68+
69+
private static void checkFunctionReturns(SubscriptionContext ctx, FunctionDef functionDef) {
70+
ReturnStatementVisitor visitor = new ReturnStatementVisitor(ctx);
71+
functionDef.body().accept(visitor);
72+
visitor.getProblematicReturns().forEach(returnStmt ->
73+
ctx.addIssue(returnStmt, MESSAGE)
74+
);
75+
}
76+
77+
private static boolean isErrorHandlerFunction(FunctionDef functionDef, SubscriptionContext ctx) {
78+
return functionDef.decorators().stream()
79+
.anyMatch(decorator -> isErrorHandlerDecorator(decorator, ctx));
80+
}
81+
82+
private static boolean isErrorHandlerDecorator(Decorator decorator, SubscriptionContext ctx) {
83+
Expression expression = decorator.expression();
84+
if (!(expression instanceof CallExpression callExpr)) {
85+
return false;
86+
}
87+
return TypeMatchers.any(
88+
TypeMatchers.isType(FLASK_APP_ERRORHANDLER_FQN),
89+
TypeMatchers.isType(BLUEPRINT_ERRORHANDLER_FQN),
90+
TypeMatchers.isType(BLUEPRINT_APP_ERRORHANDLER_FQN)
91+
).isTrueFor(callExpr.callee(), ctx);
92+
}
93+
94+
private static class ReturnStatementVisitor extends BaseTreeVisitor {
95+
private final SubscriptionContext ctx;
96+
private final List<ReturnStatement> problematicReturns = new ArrayList<>();
97+
98+
public ReturnStatementVisitor(SubscriptionContext ctx) {
99+
this.ctx = ctx;
100+
}
101+
102+
@Override
103+
public void visitReturnStatement(ReturnStatement returnStatement) {
104+
List<Expression> expressions = returnStatement.expressions();
105+
106+
if (expressions.isEmpty()) {
107+
problematicReturns.add(returnStatement);
108+
return;
109+
}
110+
111+
if (expressions.size() >= 2) {
112+
// Has status code
113+
return;
114+
}
115+
116+
Expression returnValue = expressions.get(0);
117+
118+
checkReturnedValue(returnStatement, returnValue);
119+
}
120+
121+
private void checkReturnedValue(ReturnStatement returnStatement, Expression returnValue) {
122+
if (returnValue.is(Tree.Kind.NAME)) {
123+
Name nameExpr = (Name) returnValue;
124+
125+
// Check if status_code is set on this variable via attribute assignment
126+
if (hasStatusCodeSet(nameExpr)) {
127+
return;
128+
}
129+
130+
Optional<Expression> assignedValue = Expressions.singleAssignedNonNameValue(nameExpr);
131+
if (assignedValue.isEmpty()) {
132+
return;
133+
}
134+
135+
returnValue = assignedValue.get();
136+
if (returnValue instanceof Tuple tuple && tuple.elements().size() >= 2) {
137+
return;
138+
}
139+
}
140+
141+
if (returnValue instanceof CallExpression callExpr) {
142+
if (isProblematicFlaskFunction(callExpr)) {
143+
problematicReturns.add(returnStatement);
144+
}
145+
return;
146+
}
147+
148+
problematicReturns.add(returnStatement);
149+
}
150+
151+
private boolean isProblematicFlaskFunction(CallExpression callExpr) {
152+
// Functions that don't create proper Response objects
153+
if (TypeMatchers.any(
154+
TypeMatchers.isType(JSONIFY_FQN),
155+
TypeMatchers.isType(RENDER_TEMPLATE_FQN),
156+
TypeMatchers.isType(RENDER_TEMPLATE_STRING_FQN)
157+
).isTrueFor(callExpr.callee(), ctx)) {
158+
return true;
159+
}
160+
161+
// make_response and Response can have status as second positional arg or 'status' kwarg
162+
if (TypeMatchers.any(
163+
TypeMatchers.isType(MAKE_RESPONSE_FQN),
164+
TypeMatchers.isType(RESPONSE_FQN)
165+
).isTrueFor(callExpr.callee(), ctx)) {
166+
return TreeUtils.nthArgumentOrKeyword(1, "status", callExpr.arguments()) == null;
167+
}
168+
169+
return false;
170+
}
171+
172+
@Override
173+
public void visitFunctionDef(FunctionDef pyFunctionDefTree) {
174+
// Don't visit nested functions
175+
}
176+
177+
public List<ReturnStatement> getProblematicReturns() {
178+
return problematicReturns;
179+
}
180+
181+
/**
182+
* Check if status_code is set on the given variable via attribute assignment.
183+
* Example: response.status_code = 404
184+
*/
185+
private static boolean hasStatusCodeSet(Name nameExpr) {
186+
SymbolV2 symbol = nameExpr.symbolV2();
187+
if (symbol == null) {
188+
return false;
189+
}
190+
191+
return symbol.usages().stream()
192+
.map(UsageV2::tree)
193+
.filter(Name.class::isInstance)
194+
.map(Tree::parent)
195+
.filter(QualifiedExpression.class::isInstance)
196+
.map(QualifiedExpression.class::cast)
197+
.filter(qualifiedExpr -> "status_code".equals(qualifiedExpr.name().name()))
198+
.anyMatch(qualifiedExpr -> TreeUtils.firstAncestorOfKind(qualifiedExpr, Tree.Kind.ASSIGNMENT_STMT) != null);
199+
}
200+
}
201+
}

‎python-checks/src/main/java/org/sonar/python/checks/OpenSourceCheckList.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,7 @@ public Stream<Class<?>> getChecks() {
233233
FilePermissionsCheck.class,
234234
FileHeaderCopyrightCheck.class,
235235
FixmeCommentCheck.class,
236+
FlaskErrorHandlerStatusCheck.class,
236237
FlaskHardCodedJWTSecretKeyCheck.class,
237238
FlaskHardCodedSecretKeyCheck.class,
238239
FlaskHeadersDictAccessCheck.class,

‎python-checks/src/main/resources/org/sonar/l10n/py/rules/python/S6863.html‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
<p>This is an issue when a Flask error handler returns a response without explicitly specifying the HTTP status code.</p>
2+
<h2>Why is this an issue?</h2>
3+
<p>Flask error handlers do not automatically set the HTTP status code based on the error type they handle. When you register an error handler with
4+
<code>@app.errorhandler(404)</code>, Flask does not automatically return a 404 status code from that handler.</p>
5+
<p>Instead, Flask will return a 200 OK status code by default, unless you explicitly specify the correct status code in the response. This creates a
6+
mismatch between the intended error condition and the actual HTTP response.</p>
7+
<p>This behavior can break client applications that rely on HTTP status codes to handle different error scenarios. For example, a client expecting a
8+
404 status code to handle "not found" cases will not work correctly if the server returns 200 OK instead.</p>
9+
<p>The Flask documentation explicitly states: "The status code of the response will not be set to the handler’s code. Make sure to provide the
10+
appropriate HTTP status code when returning a response from a handler."</p>
11+
<h3>What is the potential impact?</h3>
12+
<p>Client applications may not handle errors correctly, leading to unexpected behavior. API consumers might not recognize error conditions, and
13+
automated tools or frameworks that depend on proper HTTP status codes may malfunction.</p>
14+
<h2>How to fix it in Flask</h2>
15+
<p>Add the appropriate HTTP status code as a second element in the return tuple. The status code should match the error type handled by the
16+
decorator.</p>
17+
<h3>Code examples</h3>
18+
<h4>Noncompliant code example</h4>
19+
<pre data-diff-id="1" data-diff-type="noncompliant">
20+
@app.errorhandler(404)
21+
def page_not_found(e):
22+
return render_template('404.html') # Noncompliant
23+
</pre>
24+
<h4>Compliant solution</h4>
25+
<pre data-diff-id="1" data-diff-type="compliant">
26+
@app.errorhandler(404)
27+
def page_not_found(e):
28+
return render_template('404.html'), 404
29+
</pre>
30+
<p>For error handlers returning JSON responses, also include the status code explicitly.</p>
31+
<h4>Noncompliant code example</h4>
32+
<pre data-diff-id="2" data-diff-type="noncompliant">
33+
@app.errorhandler(500)
34+
def internal_error(e):
35+
return jsonify(error="Internal server error") # Noncompliant
36+
</pre>
37+
<h4>Compliant solution</h4>
38+
<pre data-diff-id="2" data-diff-type="compliant">
39+
@app.errorhandler(500)
40+
def internal_error(e):
41+
return jsonify(error="Internal server error"), 500
42+
</pre>
43+
<p>When using <code>register_error_handler()</code>, the same principle applies - always specify the status code.</p>
44+
<h4>Noncompliant code example</h4>
45+
<pre data-diff-id="3" data-diff-type="noncompliant">
46+
def handle_bad_request(e):
47+
return 'Bad request!' # Noncompliant
48+
49+
app.register_error_handler(400, handle_bad_request)
50+
</pre>
51+
<h4>Compliant solution</h4>
52+
<pre data-diff-id="3" data-diff-type="compliant">
53+
def handle_bad_request(e):
54+
return 'Bad request!', 400
55+
56+
app.register_error_handler(400, handle_bad_request)
57+
</pre>
58+
<h2>Resources</h2>
59+
<h3>Documentation</h3>
60+
<p>Flask Error Handling Documentation - <a href="https://flask.palletsprojects.com/en/stable/errorhandling/">Official Flask documentation on handling
61+
application errors and registering error handlers</a></p>
62+

‎python-checks/src/main/resources/org/sonar/l10n/py/rules/python/S6863.json‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
{
2+
"title": "Flask error handlers should set HTTP status code",
3+
"type": "BUG",
4+
"status": "ready",
5+
"remediation": {
6+
"func": "Constant\/Issue",
7+
"constantCost": "5min"
8+
},
9+
"tags": [
10+
"flask",
11+
"best-practice"
12+
],
13+
"defaultSeverity": "Major",
14+
"ruleSpecification": "RSPEC-6863",
15+
"sqKey": "S6863",
16+
"scope": "Main",
17+
"quickfix": "infeasible",
18+
"code": {
19+
"impacts": {
20+
"RELIABILITY": "MEDIUM"
21+
},
22+
"attribute": "DISTINCT"
23+
}
24+
}

‎python-checks/src/main/resources/org/sonar/l10n/py/rules/python/Sonar_way_profile.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,7 @@
233233
"S6795",
234234
"S6796",
235235
"S6799",
236+
"S6863",
236237
"S6882",
237238
"S6883",
238239
"S6887",

‎python-checks/src/test/java/org/sonar/python/checks/FlaskErrorHandlerStatusCheckTest.java‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
/*
2+
* SonarQube Python Plugin
3+
* Copyright (C) 2011-2025 SonarSource Sàrl
4+
* mailto:info AT sonarsource DOT com
5+
*
6+
* This program is free software; you can redistribute it and/or
7+
* modify it under the terms of the Sonar Source-Available License Version 1, as published by SonarSource SA.
8+
*
9+
* This program is distributed in the hope that it will be useful,
10+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
11+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
12+
* See the Sonar Source-Available License for more details.
13+
*
14+
* You should have received a copy of the Sonar Source-Available License
15+
* along with this program; if not, see https://sonarsource.com/license/ssal/
16+
*/
17+
package org.sonar.python.checks;
18+
19+
import org.junit.jupiter.api.Test;
20+
import org.sonar.python.checks.utils.PythonCheckVerifier;
21+
22+
class FlaskErrorHandlerStatusCheckTest {
23+
24+
@Test
25+
void test() {
26+
PythonCheckVerifier.verify("src/test/resources/checks/flaskErrorHandlerStatus.py", new FlaskErrorHandlerStatusCheck());
27+
}
28+
}

0 commit comments

Comments
 (0)