New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

decodeCertificate throws IllegalArgumentException on an unparseable certificate #16

Closed
scantor opened this Issue Feb 3, 2015 · 1 comment

Comments

Projects
None yet
2 participants
@scantor

scantor commented Feb 3, 2015

The org.cryptacular.util.CertUtil.decodeCertificate(byte[]) method catches the Java runtime's CertificateException when there's a problem parsing a certificate, and turns it into an IllegalArgumentException. That obviously isn't declared on the method since it's a runtime exception, so we weren't catching that in our X509Support class in OpenSAML.

I'd say as a general matter the parsing oriented methods ought to avoid throwing runtime errors since those are certainly likely to show up at runtime through no fault of the code.

@serac serac added this to the 1.1.0 milestone Jul 14, 2015

@serac

This comment has been minimized.

Member

serac commented Jul 14, 2015

Fixed in the context of pull request #22. It's worth noting that I didn't take the "no runtime exception" route because it would have had dramatic consequences on the API. Instead, I took care to declare specific subclasses of RuntimeException that are well documented and declared on the throws clause to help emphasize what exceptions can be emitted from various method calls. Then implementers have the choice of handling them or not based on their specific needs. The PR has further justification for handling all API-specific errors as runtime exceptions.

@serac serac closed this Jul 14, 2015

@serac serac added the enhancement label Jul 14, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment