3

findbugs reports these bugs about my project code.

class channelBean defines non-transient non-serializable instance field subscriptionDao
in ChannelBean.java
Field com.derbyware.qtube.beans.ChannelBean.subscriptionDao
Actual type com.derbyware.qtube.dao.SubscriptionDao 

Code:

@Named
@ViewScoped
public class ChannelBean extends BaseBacking implements Serializable {
    private static final long serialVersionUID = 1L;

    @EJB
    private SubscriptionDao subscriptionDao;

Why it says that my EJB should be serializable? I never come across such recommendation before.

AND

getCorrectAnswerTwo() May expose internal representation by returning reference to mutable object Code:

public String[] getCorrectAnswerTwo() {
        return correctAnswerTwo;
    }

I need to display the array in jsf pages. So Why the tool reports that this is a problem.

AND

setCorrectAnswers May expose internal representation by incorporating reference to mutable object

public void setCorrectAnswers(String[] correctAnswers) {
        this.correctAnswers = correctAnswers;
    }

AND

it says I should use Integer.parseInt() instead of Integer.valueOf(). Why is that?

usertest
  • 2,140
  • 4
  • 32
  • 51
  • 3
    Consider asking one question per question please. – GhostCat Dec 23 '18 at 10:33
  • in in a web enviroment you can ignore the warning for #setCorrectAnswers as each user has its own ChannelBean instance (ViewScoped) – Steve Dec 24 '18 at 12:37
  • See https://stackoverflow.com/questions/18954873/malicious-code-vulnerability-may-expose-internal-representation-by-incorporati – Raedwald Jan 09 '19 at 17:07

2 Answers2

2

You explicitly declare the containing class to implement Serializeable.

So having fields that will cause serialization to fail are probably a problem.

And the method returns the original array, so any caller of that method can then change state of that internal implementation detail.

For the difference between these two methods, simply do some research, like reading Difference between parseInt and valueOf in java?

That is all there is to this.

GhostCat
  • 137,827
  • 25
  • 176
  • 248
2

Your class ChannelBean implements Serializable. In order for a class (or better: an object of that class) to be serializable, all its fields must be serializable as well. FindBugs warns you that one field of your class ChannelBean is not serializable, in this case your EJB SubscriptionDao. In case you would ever try to serialize a ChannelBean, it will very likely result in a runtime exception as it would not be able to serialize it due to the EJB not being serializable.

To fix it, either make SubscriptionDao serializable, or make ChannelBean not implement Serializable.


expose internal representation: You directly return the array. Any receiver of that array could overwrite the values in it, e.g.:

String[] answers = object.getCorrectAnswers();
answers[0] = "My Answer";

now, "My Answer" would be a correct answer AND it would be returned in future calls to getCorrectAnswer().

The case with setCorrectAnswer() is similar:

String[] answers = new String[]{"Foo"}; object.setCorrectAnswers(answers); answers[0] = "Bar";

Now, "Bar" would be the correct answer.

To fix is, it's usually best to store a copy/clone of an array, so it cannot be modified anymore from the outside.


Integer.valueOf() creates a new object, while Integer.parseInt() does not. So the second is minimally more efficient as it does not have the overhead of memory allocation. (although a good JVM might optimize it away, so the difference would likely not be measurable, but it's still good use to prefer parseInt).

cello
  • 5,356
  • 3
  • 23
  • 28
  • "all its fields must be serializable as well" they can alternatively be null. But that's not a whole lot of use. – Andy Turner Dec 23 '18 at 10:51