Skip to content

Commit

Permalink
SONARJAVA-5523 Update rule metadata (#4965)
Browse files Browse the repository at this point in the history
  • Loading branch information
dorian-burihabwa-sonarsource authored Dec 17, 2024
1 parent 76c87a6 commit 7636762
Show file tree
Hide file tree
Showing 10 changed files with 67 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,20 @@ <h2>Why is this an issue?</h2>
<p>For all those reasons, a URI should never be hard coded. Instead, it should be replaced by a customizable parameter.</p>
<p>Further, even if the elements of a URI are obtained dynamically, portability can still be limited if the path delimiters are hard-coded.</p>
<p>This rule raises an issue when URIs or path delimiters are hard-coded.</p>
<h3>Exceptions</h3>
<p>This rule does not raise an issue when:</p>
<ul>
<li> A constant path is relative and contains at most two parts. </li>
<li> A constant path is used in an annotation </li>
<li> A path is annotated </li>
</ul>
<h2>How to fix it</h2>
<h3>Code examples</h3>
<h4>Noncompliant code example</h4>
<pre data-diff-id="1" data-diff-type="noncompliant">
public class Foo {
public static final String FRIENDS_ENDPOINT = "/user/friends"; // Compliant path is relative and has only two parts

public Collection&lt;User&gt; listUsers() {
File userList = new File("/home/mylogin/Dev/users.txt"); // Noncompliant
Collection&lt;User&gt; users = parse(userList);
Expand All @@ -40,4 +49,19 @@ <h4>Compliant solution</h4>
}
}
</pre>
<p>Exceptions examples:</p>
<pre>
public class Foo {
public static final String FRIENDS_ENDPOINT = "/user/friends"; // Compliant path is relative and has only two parts

public static final String ACCOUNT = "/account/group/list.html"; // Compliant path is used in an annotation

@Value("${base.url}" + ACCOUNT)
private String groupUrl;

@MyAnnotation()
String path = "/default/url/for/site"; // Compliant path is annotated

}
</pre>

Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,19 @@ <h4>Compliant solution</h4>
OPTION_TWO;
}
</pre>
<h3>Exceptions</h3>
<p>The rule applies to fields of primitive types (for example, <code>float</code>), boxed primitives (<code>Float</code>), and Strings. We do not
apply it to other types, which can be mutated, or have methods with side effects.</p>
<pre>
public static final Logger log = getLogger(MyClass.class);
public static final List&lt;Integer&gt; myList = new ArrayList&lt;&gt;();

// call with side-effects
log.info("message")

// mutating an object
myList.add(28);
</pre>
<h2>Resources</h2>
<h3>External coding guidelines</h3>
<ul>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ <h4>Noncompliant code example</h4>
<h4>Compliant solution</h4>
<pre data-diff-id="1" data-diff-type="compliant">
public class MyClass {
String s = ""; // Noncompliant
String s = ""; // Compliant
}
</pre>
<h2>Resources</h2>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@
"func": "Constant\/Issue",
"constantCost": "5min"
},
"tags": [],
"tags": [
"convention"
],
"defaultSeverity": "Blocker",
"ruleSpecification": "RSPEC-1451",
"sqKey": "S1451",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,10 @@
"func": "Constant\/Issue",
"constantCost": "5min"
},
"tags": [],
"tags": [
"confusing",
"bad-practice"
],
"defaultSeverity": "Major",
"ruleSpecification": "RSPEC-1751",
"sqKey": "S1751",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
<p>This rule raises an issue on a non-transient and non-serializable field within a serializable class, if said class does not have
<code>writeObject</code> and <code>readObject</code> methods defined.</p>
<h2>Why is this an issue?</h2>
<p>By contract, fields in a <code>Serializable</code> class must themselves be either <code>Serializable</code> or <code>transient</code>. Even if the
class is never explicitly serialized or deserialized, it is not safe to assume that this cannot happen. For instance, under load, most J2EE
application frameworks flush objects to disk.</p>
<p>By contract, non-static fields in a <code>Serializable</code> class must themselves be either <code>Serializable</code> or <code>transient</code>.
Even if the class is never explicitly serialized or deserialized, it is not safe to assume that this cannot happen. For instance, under load, most
J2EE application frameworks flush objects to disk.</p>
<p>An object that implements <code>Serializable</code> but contains non-transient, non-serializable data members (and thus violates the contract)
could cause application crashes and open the door to attackers. In general, a <code>Serializable</code> class is expected to fulfil its contract and
not exhibit unexpected behaviour when an instance is serialized.</p>
Expand Down Expand Up @@ -84,6 +84,16 @@ <h2>How to fix it</h2>
}
}
</pre>
<p>Finally, static fields are out of scope for serialization, so making a field static prevents issues from being raised.</p>
<pre>
public class Person implements Serializable {
private static final long serialVersionUID = 1905122041950251207L;

private String name;

private static Logger log = getLogger(); // Compliant, static fields are not serialized
}
</pre>
<h2>Resources</h2>
<ul>
<li> CWE - <a href="https://cwe.mitre.org/data/definitions/594">CWE-594 - Saving Unserializable Objects to Disk</a> </li>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<p>Formatted SQL queries can be difficult to maintain, debug and can increase the risk of SQL injection when concatenating untrusted values into the
query. However, this rule doesn’t detect SQL injections (unlike rule {rule:javasecurity:S3649}), the goal is only to highlight complex/formatted queries.</p>
query. However, this rule doesn’t detect SQL injections (unlike rule {rule:java:S3649}), the goal is only to highlight complex/formatted queries.</p>
<h2>Ask Yourself Whether</h2>
<ul>
<li> Some parts of the query come from untrusted values (like user inputs). </li>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ <h2>Why is this an issue?</h2>
<li> <code>org.springframework.beans.factory.annotation.Value</code> </li>
<li> <code>javax.annotation.Inject</code> </li>
<li> <code>javax.annotation.Resource</code> </li>
<li> <code>javax.persistence.PersistenceContext</code> </li>
<li> <code>jakarta.annotation.Resource</code> </li>
<li> <code>jakarta.inject.Inject</code> </li>
<li> <code>jakarta.persistence.PersistenceContext</code> </li>
</ul>
<h2>How to fix it</h2>
<p>Add one of these annotations to all non-<code>static</code> members: <code>@Resource</code>, <code>@Inject</code>, <code>@Autowired</code> or
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@
"func": "Constant\/Issue",
"constantCost": "2min"
},
"tags": [],
"tags": [
"confusing"
],
"defaultSeverity": "Major",
"ruleSpecification": "RSPEC-3981",
"sqKey": "S3981",
Expand Down
2 changes: 1 addition & 1 deletion sonarpedia.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"languages": [
"JAVA"
],
"latest-update": "2024-11-29T11:04:25.911576775Z",
"latest-update": "2024-12-17T09:08:48.208626612Z",
"options": {
"no-language-in-filenames": true,
"preserve-filenames": false
Expand Down

0 comments on commit 7636762

Please sign in to comment.