2

I've a method which takes two arguments, and populates arg1 from arg2. The population logic calls upon a method which may throw an extended Exception and probably a NumberFormatException too if the object has unexpected values. The method logic is:

public Type1 populateType1FromType2(Type1 arg1, Type2 arg2) {
    if(arg2 is null or empty) {
        return arg1;
    }
    try {
        //invoke setters on arg1 and populate values from arg2 //may lead to NPE here or in hierarchy
        //statements
    } catch(Exception e) { //Issue 1
        //log message that this population failed along with exception trace //Issue 2
    }
    return arg1; //may be null, may be half populated, may be fully populated if no exception above
}

Now after running Fortify, I am being reported about the issues in above snippet. I understand that it defeats the purpose of typed exceptions. But at the same point I am not sure how to handle this issue because any exception comes, we catch it (Issue 1) we basically log it and inform that its failed (Issue 2). I can't catch NPE, its nonsensical if I explicitly mention.

Fortify report and expectation:

Issue 1 report: Do not catch broad exception classes like Exception, Throwable, Error, or except at the very top level of the program or thread (Because we are catching Exception object)

Issue 1 expectation: The application should not fail even if its a null pointer, just the method will not work and we will still proceed with other parts of the application logic.

Issue 2 report: The function might reveal system data or debugging information. (Because we are adding exception trace too)

Issue 2 expectation: In case of any exception, we will log the trace, thats expected behavior, it does not contain any customer sensitive information, rather internal ids and we need that.

What to do?

xploreraj
  • 3,792
  • 12
  • 33
  • 51
  • What do you actually want the program to do, specifically? – Dziugas Jul 05 '17 at 09:47
  • Are you checking for null pointer in arg1? – Atspulgs Jul 05 '17 at 09:55
  • "I can't catch NPE, its nonsensical if I explicitly mention" - Does not make any sense. Why not use Optional<> as return type? Why do you think catching NPE is nonsensical? – Prashant Jul 05 '17 at 09:56
  • The way to handle this, if you want the method to continue, is either by preventing npe by checking for null before passing the value in or simply wrap in a try-catch only the line where npe may occur. – Dziugas Jul 05 '17 at 09:57
  • I have not used it, its java7 too! – xploreraj Jul 05 '17 at 09:58
  • @Dziugas If its a NPE anywhere I still have to fail the method, control it, log it and return `arg1` whether its null or half populated. I dont see how your idea works. – xploreraj Jul 05 '17 at 10:01
  • So in your npe catch block, log it and return null – Dziugas Jul 05 '17 at 10:02
  • I havent used Fortify so I have no idea how it warns you, but perhaps you can provide us with the exact warning or error message it is giving you? – Atspulgs Jul 05 '17 at 10:08
  • Issue 1: Do not catch broad exception classes like Exception, Throwable, Error, or except at the very top level of the program or thread (Because we are catching Exception object) Issue 2: The function might reveal system data or debugging information. (Because we are adding exception trace too) – xploreraj Jul 05 '17 at 10:24

0 Answers0