Exceptions thrown from com.nimbusds.jwt.proc.DefaultJWTProcessor has incorrect stack traces

Issue #229 resolved
Christoffer Eide created an issue

In DefaultJWTProcessor, the exceptions are declared as constants.

private static final BadJOSEException PLAIN_JWT_REJECTED_EXCEPTION =
        new BadJOSEException("Unsecured (plain) JWTs are rejected, extend class to handle");

These exceptions are then thrown:

@Override
public JWTClaimsSet process(final PlainJWT plainJWT, final C context)
    throws BadJOSEException, JOSEException {

    verifyAndReturnClaims(plainJWT, context); // just check claims, no return

    throw PLAIN_JWT_REJECTED_EXCEPTION;
}

This messes up the stack traces:

#!

com.nimbusds.jose.proc.BadJOSEException: Unsecured (plain) JWTs are rejected, extend class to handle
    at com.nimbusds.jwt.proc.DefaultJWTProcessor.<clinit>(DefaultJWTProcessor.java:90)
    at com.vimond.common.jwt.JwtTokenVerifier.<init>(JwtTokenVerifier.java:90)
    at com.vimond.common.jwt.JwtTokenVerifier.<init>(JwtTokenVerifier.java:42)
    at com.vimond.common.jwt.JwtTokenVerifier$Builder.build(JwtTokenVerifier.java:267)

The stack trace does not tell me where in the code the exception was thrown.

Comments (6)

  1. Vladimir Dzhuvinov

    The constants were put there for performance reasons, for exception catching is rather expensive, and these exceptions can appear quite often. You're right that the stack traces will appear incorrect as result of that.

  2. Pierre Viret

    Can you please set a higher priority to this issue ? This is a real problem for us to have a wrong stack trace for the thrown exceptions. We loose time during problem analysis as the stack trace tells us that the error occured during initialization of the Nimbus Library and not during execution. OMHO it's a bad practive to create exception as static constant during initialization of the class. A simple fix would be to create the text message for the exception as constant and to use the message constant when the exception is created and thrown.

    I would willingly contribute to this fix, but it seems that I cannot create a pull request.

  3. Pierre Viret

    Here my contribution: I have updated the file "src/main/java/com/nimbusds/jwt/proc/DefaultJWTProcessor.java", it's a trivial fix where I have replaced the Exception contatns with String constants, and the exceptions are created at the time they get thrown. As I don't have privilege to create a pull request, I simply put the "git diff" output here. I hope this helps.

    diff --git a/src/main/java/com/nimbusds/jwt/proc/DefaultJWTProcessor.java b/src/main/java/com/nimbusds/jwt/proc/DefaultJWTProcessor.java
    index df081ec5..e02e0a83 100644
    --- a/src/main/java/com/nimbusds/jwt/proc/DefaultJWTProcessor.java
    +++ b/src/main/java/com/nimbusds/jwt/proc/DefaultJWTProcessor.java
    @@ -86,28 +86,28 @@ public class DefaultJWTProcessor<C extends SecurityContext>
        implements ConfigurableJWTProcessor<C> {
    
        // Cache exceptions
    -   private static final BadJOSEException PLAIN_JWT_REJECTED_EXCEPTION =
    -       new BadJOSEException("Unsecured (plain) JWTs are rejected, extend class to handle");
    -   private static final BadJOSEException NO_JWS_KEY_SELECTOR_EXCEPTION =
    -       new BadJOSEException("Signed JWT rejected: No JWS key selector is configured");
    -   private static final BadJOSEException NO_JWE_KEY_SELECTOR_EXCEPTION =
    -       new BadJOSEException("Encrypted JWT rejected: No JWE key selector is configured");
    -   private static final JOSEException NO_JWS_VERIFIER_FACTORY_EXCEPTION =
    -       new JOSEException("No JWS verifier is configured");
    -   private static final JOSEException NO_JWE_DECRYPTER_FACTORY_EXCEPTION =
    -       new JOSEException("No JWE decrypter is configured");
    -   private static final BadJOSEException NO_JWS_KEY_CANDIDATES_EXCEPTION =
    -       new BadJOSEException("Signed JWT rejected: Another algorithm expected, or no matching key(s) found");
    -   private static final BadJOSEException NO_JWE_KEY_CANDIDATES_EXCEPTION =
    -       new BadJOSEException("Encrypted JWT rejected: Another algorithm expected, or no matching key(s) found");
    -   private static final BadJOSEException INVALID_SIGNATURE =
    -       new BadJWSException("Signed JWT rejected: Invalid signature");
    -   private static final BadJWTException INVALID_NESTED_JWT_EXCEPTION =
    -       new BadJWTException("The payload is not a nested signed JWT");
    -   private static final BadJOSEException NO_MATCHING_VERIFIERS_EXCEPTION =
    -       new BadJOSEException("JWS object rejected: No matching verifier(s) found");
    -   private static final BadJOSEException NO_MATCHING_DECRYPTERS_EXCEPTION =
    -       new BadJOSEException("Encrypted JWT rejected: No matching decrypter(s) found");
    +   private static final String PLAIN_JWT_REJECTED_EXCEPTION =
    +       "Unsecured (plain) JWTs are rejected, extend class to handle";
    +   private static final String NO_JWS_KEY_SELECTOR_EXCEPTION =
    +       "Signed JWT rejected: No JWS key selector is configured";
    +   private static final String NO_JWE_KEY_SELECTOR_EXCEPTION =
    +       "Encrypted JWT rejected: No JWE key selector is configured";
    +   private static final String NO_JWS_VERIFIER_FACTORY_EXCEPTION =
    +       "No JWS verifier is configured";
    +   private static final String NO_JWE_DECRYPTER_FACTORY_EXCEPTION =
    +       "No JWE decrypter is configured";
    +   private static final String NO_JWS_KEY_CANDIDATES_EXCEPTION =
    +       "Signed JWT rejected: Another algorithm expected, or no matching key(s) found";
    +   private static final String NO_JWE_KEY_CANDIDATES_EXCEPTION =
    +       "Encrypted JWT rejected: Another algorithm expected, or no matching key(s) found";
    +   private static final String INVALID_SIGNATURE =
    +       "Signed JWT rejected: Invalid signature";
    +   private static final String INVALID_NESTED_JWT_EXCEPTION =
    +       "The payload is not a nested signed JWT";
    +   private static final String NO_MATCHING_VERIFIERS_EXCEPTION =
    +       "JWS object rejected: No matching verifier(s) found";
    +   private static final String NO_MATCHING_DECRYPTERS_EXCEPTION =
    +       "Encrypted JWT rejected: No matching decrypter(s) found";
    
        /**
         * The JWS key selector.
    @@ -303,7 +303,7 @@ public class DefaultJWTProcessor<C extends SecurityContext>
    
            verifyAndReturnClaims(plainJWT, context); // just check claims, no return
    
    -       throw PLAIN_JWT_REJECTED_EXCEPTION;
    +       throw new BadJOSEException(PLAIN_JWT_REJECTED_EXCEPTION);
        }
    
    
    @@ -313,17 +313,17 @@ public class DefaultJWTProcessor<C extends SecurityContext>
    
            if (getJWSKeySelector() == null) {
                // JWS key selector may have been deliberately omitted
    -           throw NO_JWS_KEY_SELECTOR_EXCEPTION;
    +           throw new BadJOSEException(NO_JWS_KEY_SELECTOR_EXCEPTION);
            }
    
            if (getJWSVerifierFactory() == null) {
    -           throw NO_JWS_VERIFIER_FACTORY_EXCEPTION;
    +           throw new JOSEException(NO_JWS_VERIFIER_FACTORY_EXCEPTION);
            }
    
            List<? extends Key> keyCandidates = getJWSKeySelector().selectJWSKeys(signedJWT.getHeader(), context);
    
            if (keyCandidates == null || keyCandidates.isEmpty()) {
    -           throw NO_JWS_KEY_CANDIDATES_EXCEPTION;
    +           throw new BadJOSEException(NO_JWS_KEY_CANDIDATES_EXCEPTION);
            }
    
            ListIterator<? extends Key> it = keyCandidates.listIterator();
    @@ -344,11 +344,11 @@ public class DefaultJWTProcessor<C extends SecurityContext>
    
                if (! it.hasNext()) {
                    // No more keys to try out
    -               throw INVALID_SIGNATURE;
    +               throw new BadJWSException(INVALID_SIGNATURE);
                }
            }
    
    -       throw NO_MATCHING_VERIFIERS_EXCEPTION;
    +       throw new BadJOSEException(NO_MATCHING_VERIFIERS_EXCEPTION);
        }
    
    
    @@ -358,17 +358,17 @@ public class DefaultJWTProcessor<C extends SecurityContext>
    
            if (getJWEKeySelector() == null) {
                // JWE key selector may have been deliberately omitted
    -           throw NO_JWE_KEY_SELECTOR_EXCEPTION;
    +           throw new BadJOSEException(NO_JWE_KEY_SELECTOR_EXCEPTION);
            }
    
            if (getJWEDecrypterFactory() == null) {
    -           throw NO_JWE_DECRYPTER_FACTORY_EXCEPTION;
    +           throw new JOSEException(NO_JWE_DECRYPTER_FACTORY_EXCEPTION);
            }
    
            List<? extends Key> keyCandidates = getJWEKeySelector().selectJWEKeys(encryptedJWT.getHeader(), context);
    
            if (keyCandidates == null || keyCandidates.isEmpty()) {
    -           throw NO_JWE_KEY_CANDIDATES_EXCEPTION;
    +           throw new BadJOSEException(NO_JWE_KEY_CANDIDATES_EXCEPTION);
            }
    
            ListIterator<? extends Key> it = keyCandidates.listIterator();
    @@ -402,7 +402,7 @@ public class DefaultJWTProcessor<C extends SecurityContext>
    
                    if (signedJWTPayload == null) {
                        // Cannot parse payload to signed JWT
    -                   throw INVALID_NESTED_JWT_EXCEPTION;
    +                   throw new BadJWTException(INVALID_NESTED_JWT_EXCEPTION);
                    }
    
                    return process(signedJWTPayload, context);
    @@ -411,6 +411,6 @@ public class DefaultJWTProcessor<C extends SecurityContext>
                return verifyAndReturnClaims(encryptedJWT, context);
            }
    
    -       throw NO_MATCHING_DECRYPTERS_EXCEPTION;
    +       throw new BadJOSEException(NO_MATCHING_DECRYPTERS_EXCEPTION);
        }
     }
    
  4. Log in to comment