Bug 533376 - Inconsistent error return values, CMSServlet.outputError() should be removed
Summary: Inconsistent error return values, CMSServlet.outputError() should be removed
Keywords:
Status: CLOSED EOL
Alias: None
Product: Dogtag Certificate System
Classification: Retired
Component: Certificate Manager
Version: 1.2
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: RHCS Maintainers
QA Contact: Ben Levenson
URL:
Whiteboard:
Depends On:
Blocks: 688231
TreeView+ depends on / blocked
 
Reported: 2009-11-06 13:28 UTC by John Dennis
Modified: 2020-03-27 20:05 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2020-03-27 20:05:10 UTC
Embargoed:


Attachments (Terms of Use)

Description John Dennis 2009-11-06 13:28:50 UTC
This bug is related to bug #531937

CMS has two distinct ways it reports errors, (see bug #531937 for an
explanation). CMSRequest.setStatus() & CMSRequest.setError()
vs. CMSServlet.outputError()

In theory CMSServlet.outputError() should only be called when xml output is
requested, however several routines which use CMSServlet.outputError() fail to
check for the xml flag. The CMSRequest.setStatus() & CMSRequest.setError()
methodology always checks the xml flag because the check is performed in the
CMSServlet code after the servlet process method returns.

The error code from the two methodologies have *CONFLICTING* sets of return
values:

These values are returned when CMSServlet.outputError is invoked:

SUCCESS      = "0";
FAILURE      = "1";
AUTH_FAILURE = "2";

These values are returned when CMSRequest.setStatus() is invoked:

UNAUTHORIZED = 1
SUCCESS      = 2
PENDING      = 3
SVC_PENDING  = 4
REJECTED     = 5
ERROR        = 6
EXCEPTION    = 7

Note how in the first case SUCCESS is 0 but in the second case it's 2. In the
first case ERROR is 1 and in the second case it's 6. In the first case
UNAUTHORIZED is 2 and in the second case it's 1.

So does a return code of 2 mean SUCCESS or UNAUTHORIZED? To answer that question
you have to know which servlet was invoked and how it handles errors. That's
difficult and error prone.

When CMSServlet.outputError is invoked it's often done inconsistently resulting
in different error codes for the same error condition. To understand how this
happens you need to understand that CMSServlet.outputError() has multiple
signatures, with and without the status parameter. If CMSServlet.outputError()
is invoked without the status parameter it defaults to FAILURE ("1").

Here are some examples of CMSServlet.outputError() being invoked:

outputError(httpResp, "Error: Not authenticated");
outputError(httpResp, AUTH_FAILURE, "Error: Not authenticated");

Note, they are both the same error condition but in the first case the error
code will be "1" (FAILURE) but in the second case it will be "2" (AUTH_FAILURE)!

Also, there is a distinction between authentication and authorization
failures. This is never reflected in the error code, it would be really nice if
the error code made clear the distinction between authentication and
authorization failures. The error message returned does draw the distinction
between authentication and authorization, but the actual strings used are
inconsistent, you could search the string for "authen" vs. "author" as work
around given the inconsistent messages, however that won't work if the messages get localized as they should.

Many of the calls to CMSServlet.outputError() fail to use localized strings
(e.g. calling CMS.getUserMessage()), instead they just output hardcoded
English.

I think most of the problems noted above can be resolved by eliminating the use
of CMSServlet.outputError() and instead use the CMSRequest.setStatus() &
CMSRequest.setError() methodology, which by the way is the predominate
methodolgy by a long shot.

Comment 1 John Dennis 2009-11-06 15:21:21 UTC
There is another problem with the use of CMSServlet.outputError(). Normally the error codes returned by this function are from the set:

SUCCESS      = "0";
FAILURE      = "1";
AUTH_FAILURE = "2";

However in pki/base/common/src/com/netscape/cms/servlet/profile/ProfileSubmitServlet.java

The following done:

outputError(response, errorCode, errorReason, requestIds);

However, the variable errorCode have these potential values:

EXCEPTION = "1"
DEFERRED  = "2"
REJECTED  = "3"

So is 2 AUTH_FAILURE or DEFERRED? Is 1 FAILURE or EXCEPTION? By the way we already have a different error code for EXCEPTION when returned as a status (7)

This is in addition to all the other uses of CMSServlet.outputError in the *same* routine which use the first set of values.

Comment 2 Dmitri Pal 2009-11-06 18:43:10 UTC
Andrew and Christina, John can fix this bug but he needs some guidance on the best approach since there might be a broader impact. Please get together to discuss.


Note You need to log in before you can comment on or make changes to this bug.