-
Notifications
You must be signed in to change notification settings - Fork 0
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
- Loading branch information
1 parent
b820b34
commit 86b2022
Showing
4 changed files
with
122 additions
and
5 deletions.
There are no files selected for viewing
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
111 changes: 111 additions & 0 deletions
111
content/posts/2024-04-29-handling-ei-expose-rep/index.mdx
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,111 @@ | ||
--- | ||
title: Handling EI_EXPOSE_REP & EI_EXPOSE_REP2 | ||
author: Sudaraka Jayathilaka | ||
date: 2024-04-29 | ||
hero: ./images/hero.png | ||
excerpt: SpotBugs is a great tool for static code analysis. It can help you find bugs, security vulnerabilities, and performance issues in your Java code. | ||
--- | ||
|
||
[SpotBugs](https://spotbugs.github.io/) is a great tool for static code analysis. Recently I got two similar warnings in one of the codebases I work on | ||
and I had to fix it. | ||
|
||
## EI_EXPOSE_REP & EI_EXPOSE_REP2 Warning definitions | ||
|
||
You can find the official documentation for these two warnings in the SpotBugs documentation. | ||
- Warning [EI_EXPOSE_REP](https://spotbugs.readthedocs.io/en/stable/bugDescriptions.html#ei-expose-rep) | ||
- Warning [EI_EXPOSE_REP2](https://spotbugs.readthedocs.io/en/stable/bugDescriptions.html#ei-expose-rep2) | ||
|
||
## EI_EXPOSE_REP | ||
|
||
This warning is mainly about exposing a mutable object from a class. Let's take a class which has a | ||
mutable object such as a `List`. | ||
|
||
```java | ||
public class MyClass { | ||
private List<String> myList; | ||
|
||
public MyClass() { | ||
myList = new ArrayList<>(); | ||
} | ||
|
||
public List<String> getMyList() { | ||
return myList; | ||
} | ||
} | ||
``` | ||
|
||
Now you can see that we are exposing the `myList` object via the `getMyList()` method. This means that anyone who has access to the | ||
`MyClass` object can modify the `myList` object. This is a clear violation of the encapsulation principle in OOP. This is where the | ||
`EI_EXPOSE_REP` warning comes in. | ||
|
||
### The Fix | ||
We can mainly fix this using two approaches. | ||
1. Return an unmodifiable list from the `getMyList()` method. You can change the getMyList() method as follows. | ||
```java | ||
public List<String> getMyList() { | ||
return Collections.unmodifiableList(myList); | ||
} | ||
``` | ||
2. Return a copy of the list from the `getMyList()` method. You can change the getMyList() method as follows. | ||
```java | ||
public List<String> getMyList() { | ||
return List.copyOf(myList); | ||
} | ||
``` | ||
|
||
## EI_EXPOSE_REP2 | ||
|
||
This warning is pretty much related to the previous warning. The only difference is that this warning is about keeping | ||
a reference to a mutable object within the class. Let's take the same example we used in the previous warning. | ||
|
||
```java | ||
public class MyClass { | ||
private List<String> myList; | ||
|
||
public MyClass(List<String> myList) { | ||
this.myList = myList; | ||
} | ||
} | ||
``` | ||
|
||
In the above example you can see how the constructor gets a reference to a mutable List from outside the class and assigns it to the | ||
`myList` field. This means that the caller can modify the `myList` object from outside of the class. This is again a clear violation of the | ||
encapsulation principle and a security risk. | ||
|
||
### The Fix | ||
|
||
The fix for this warning is slightly different from the previous warning. You can fix this by, | ||
|
||
1. Creating a copy of the list when assigning it to the `myList` variable. | ||
```java | ||
public MyClass(List<String> myList) { | ||
this.myList = List.copyOf(myList); | ||
} | ||
``` | ||
2. Creating an unmodifiable list when assigning it to the `myList` variable. | ||
```java | ||
public MyClass(List<String> myList) { | ||
this.myList = Collections.unmodifiableList(myList); | ||
} | ||
``` | ||
|
||
## Conclusion | ||
|
||
These warnings can be really helpful given the right context. But in some cases, it might be an overkill to fix these warnings. So | ||
it's always better to analyze the context and decide whether to fix these warnings or not. In case you want to ignore these warnings | ||
you can use the `@SuppressFBWarnings` annotation provided by SpotBugs. | ||
|
||
```java | ||
@SuppressFBWarnings("EI_EXPOSE_REP") | ||
public class MyClass { | ||
private List<String> myList; | ||
|
||
public MyClass() { | ||
myList = new ArrayList<>(); | ||
} | ||
|
||
public List<String> getMyList() { | ||
return myList; | ||
} | ||
} | ||
``` |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters