Should security restrictions cause a service to return null or throw an exception? [closed]

Question

I'm in a bit of a disagreement with a more experienced developer on this issue, and wondering what others think about it; our environment is Java, EJB 3, services, etc.

The code I wrote calls a service to get things and to create things. The problem I ran into was that I got null pointer exceptions that didn't make sense. For example, when I ask the service to create an object, I get null back; when I try to look up an object with a known valid ID, I get null back. I spent some time trying to figure out what was wrong in my code; since I'm less experienced I usually assume I've done something wrong, but it turns out the reason for the null returns was security. If the user principal using my service didn't have the right permissions for the target service, then it simply returns null. Most other services here aren't documented very well either, so apparently this is just something you have to know.

This is rather confusing as a developer writing code that interacts with the service. It would make much more sense to me if the service thew an exception that would tell me that the user didn't have the proper permissions to load this thing or to create that thing; I would then immediately know why my service wasn't working as expected.

The more experienced developer who wrote the service argued that asking for the data is not an error condition, and that exceptions should only be thrown in an error condition, not when the user doesn't have access to the data. This data is often looked up in a GUI, and for those users without the right permissions, these things simply "do not exist". So, in short: asking is not wrong, hence no exception. Get methods return null because to those users those things "don't exist". Create methods return null when the user wasn't allowed to create that thing.

Is this normal and/or good practice? I prefer using exceptions because I find it much easier to know what's going on. So I would for example also prefer to throw a NotFoundException if you asked for an object with an invalid ID, rather than returning null.

Answer 1

Exceptions should only be thrown when there is an error and asking for a thing is not an error.

Asking for a thing may not be an error, but not having permissions to something you asked for is surely some sort of error. Exceptions are an alternative way of reporting exceptional conditions, to be used instead of special return values (such as null, which, as you wrote, is of absolutely no help if there are > 1 possible reasons why things could go awry (even if there is exactly 1 possible reason now, there could be 2 possible reasons in the next version, so using null as a return value indicating failure would be painting yourself into corner)). If the service doesn't report in any way why it won't do what you asked for, then it's definitely bad design.

Whether to use a special return value (e.g. negative integers are useful for functions that normally return a nonnegative integer), an exception, a global error handler or logger etc., is an implementation detail in the end. The choice depends on the language, on the situation, on conventions, and is also a question of taste. The main point is that there should be some direct way of finding out why something doesn't work. Otherwise your only option is to trash around with different options and whatever to find out what correlates with the black box's behavior, and it's waste of time.

String.substring() throws an IndexOutOfBoundsException if the index is out of bounds. I can't think of any advantages to returning null instead, even though - philosophically - one could argue that a String doesn't contain anything outside its bounds, so null would be a logical return value then. Being logical-philosophical and being practical are obviously two different animals.

Answer 2

It comes down to what the contract is. If your contract says you can request something that doesn't exist, it should say what happens (null, or null object).

On the other hand, if your contract says you should call a different method first (DoesSomethingExist()) and then call the Get method, then your contract might say that you can only get things that do exist, and throw an exception if they don't. The exception message could say something like, "Make sure to call DoesSomethingExist() first" which is a useful error message.

Answer 3

There is no single one-size-fits-all answer to the question. Whether to use exceptions or return null depends on the situation.

Instead of looking at this purely from a dogmatic point of view, look at the interface as a user interface. User interfaces should be usable. So regardless of your own opinions on the "right" way to do something, pick the method that is the most usable from the perspective of someone using your interface.

If the caller needs to know why an object isn't being returned, an exception is appropriate. If, however, the general concept is "if you don't have permission, it doesn't exist", the null is acceptable.

Specifically in your case, I'd say nulls are perfectly acceptable for looking up an item if the caller is first required to log in or connect to the server. That is when you would be told that you do or don't have permission. Once you get past the gate it's reasonable that when you search for something you don't have permission to see (or even know it exists), you should get a null.

If, on the other hand, you have no initial connection or login step, an exception makes sense. If the caller knows an item exists and they aren't getting it back, the API isn't being helpful to just return a null.

For creating an item, however, that's a different story. Presumably there could be a lot of reasons for that to fail -- no permissions, bad parameters, server out of memory, etc. If the developer is given a null for all those conditions, they have no way to determine a proper course of action.

So, to answer the question, ask yourself what the most usable solution is from the perspective of someone using the service. It may be that the way you are using it is atypical, and for the most common case a null is the right answer.

So, don't get into a religious war over this, decide what's right for this particular problem.

Answer 4

I definitely think it is a bad design. Null-pointer is not a valid answer to me and it can be tricky to manage.

If the user tries to connect a service without the proper credential, I should reply with a clear and concise answer. The answer could be an exception or a special object but not null.

You should leave the null answer when the network link is broken or other unexpected critical malfunction.

Furthermore, the time you spend understanding why you got a null is a strong argument. Any piece of code should be easy to understand and use. Having a null value is not the case.

Answer 5

Bearing good software design in mind, you should think about the life of your project.
By returning null, as it has been said, you do not give any information to the client. Look what happend to you, at first you did not realize where the problem was. Moreover, if no documentation is given, this is a mess.

By throwing an exception, you can tell what went wrong. You can even customize the displayed text to be more specific if you want to.

On the other hand, by returning null you make the client investigate what it is going on.

Furthermore, you realized that there was a problem because you got elsewhere a NullPointerException. Now imagine you do not store the return of that function... You will be forced to surround each call to that function with an if else block...

From my point of view, returning null is a bad practice.

Answer 6

The more experienced developer who wrote the service argued that asking for the data is not an error condition, and that exceptions should only be thrown in an error condition, not when the user doesn't have access to the data.

From my point of view, this makes no sense.

Querying for data without permission should result in an exception, because it is an unexpected result - to get no result - without proper access.

Analogy: The responsecode for a GET without authorization is not 200 ok with no data, it's actually 401 Unauthorized.

Answer 7

Returning null isn't great. It tells you nothing. To take your example, if an app is trying to look something up and the response is null for both security problems and if the item doesn't exist, then how do you tell the difference between the two?

A meaningful response may allow the application to make logical choices about what to do next or how to resolve problems.

Answer 8

If the root cause is an unauthenticated user then I prefer a hard HTTP 401 response perhaps with a redirect to a pretty-message-page (and a notification to someone that cares). Authorization-related causes are more case-by-case; there are arguments for returning HTTP errors (403, say) and arguments for returning special well-formed codes/messages in the service response.

Exceptions should only be used in exceptional cirumstances and mean that something has gone wrong. I disagree that they should be overloaded to mean "not allowed". (The same is true for the null return value: I disagree that it should be used to mean "not allowed".)

Answer 9

To play devils advocate I will attempt to take the side of returning null.

In my opinion there is just one situation where this kind of response is nearly acceptable and that is, as in this case, when security is involved.

It is very easy to accidentally give away details of your system that unauthorized people should not know about. This should be avoided.

Take, for example, a user name and password checker. Returning a "User name not found" error and a "Wrong password" error as separate error codes would obviously open you up to serious security issues as someone could first look for a valid user name and then look for a password. Returning a generic "Invalid username/password" would be a better option.

Thus, In this particular case I would say that it is almost ok to return null but I agree, it would be very unpleasant to work with.

Answer 10

I think return null is unfriendly, when the client invoke this method and return null, the client do not know what happened and why it returned null. So we should be throw execption which is make sense and provide a documentation to descrip this execption.