> On March 7, 2016, 6:33 p.m., Jinmei Liao wrote: > > geode-core/src/main/java/com/gemstone/gemfire/security/GemFireSecurityException.java, > > line 38 > > <https://reviews.apache.org/r/44402/diff/1/?file=1281204#file1281204line38> > > > > Probably I don't understand the purpose of this yet. Why we would need > > to have a cause variable? Is the cuase in the parent class not sufficient? > > Kirk Lund wrote: > Adding a cause variable with custom serialization is how Spring works > around the NamingException (cause) with an unserializable field (resolvedObj > which is frequently LdapCtxt which is unserializable). You can't override the > way in which super.cause is serialized, so the Exception has to have its own > cause field -- super.cause remains null. During serialization, if cause is a > NamingException and if it has an unserializable field in resolvedObj, then > the Exception temporarily nulls it out and immediately restores the field > after serializing itself. > > I copied the approach from Spring Ldap which wraps all NamingExceptions, > but implemented it a little differently specific to our need. > > Jinmei Liao wrote: > I see. Thanks! If this is used to handle serialization problem, would > GemfireException be a better place for it? I saw it has some commented out > code about "cause"..... > > Kirk Lund wrote: > I took a look at GemFireException. That dead code is from early GemFire > when the JDK Throwable didn't support cause. We should delete the deadcode. > > I'd prefer to keep the custom serialization for NamingException as close > to the code that needs it as possible. It's only needed by our security > exceptions that wrap ldap NamingException. If we ever find ourselves using > non-security exceptions to wrap NamingException, then we could always move > the custom cause to a different exception class. I think that if security is > the only code section that needs this then we should solve the problem there.
Sure. Thanks! - Jinmei ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44402/#review122325 ----------------------------------------------------------- On March 4, 2016, 7:28 p.m., Kirk Lund wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/44402/ > ----------------------------------------------------------- > > (Updated March 4, 2016, 7:28 p.m.) > > > Review request for geode, Jens Deppe and Jinmei Liao. > > > Bugs: GEODE-949 > https://issues.apache.org/jira/browse/GEODE-949 > > > Repository: geode > > > Description > ------- > > GEODE-949: fix serialization of GemFireSecurityExceptions to improve debugging > > * add workarounds to security exceptions for unserializable fields (inspired > by similiar code in Spring ldap) > * add cause for security exceptions in test code instead of eating exceptions > * cleanup javadocs of security exceptions > * introduce unit tests > * replace hardcoded class strings with class getName in security test code to > help facilitate repackaging > > > Diffs > ----- > > > geode-core/src/main/java/com/gemstone/gemfire/security/GemFireSecurityException.java > 1f97420 > > geode-core/src/main/java/com/gemstone/gemfire/security/NotAuthorizedException.java > c6165a6 > > geode-core/src/test/java/com/gemstone/gemfire/security/GemFireSecurityExceptionTest.java > PRE-CREATION > > geode-core/src/test/java/com/gemstone/gemfire/security/NotAuthorizedExceptionTest.java > PRE-CREATION > geode-core/src/test/java/security/AuthzCredentialGenerator.java e15a60a > geode-core/src/test/java/security/CredentialGenerator.java 7a430f1 > geode-core/src/test/java/security/DummyAuthzCredentialGenerator.java > 7e40d13 > geode-core/src/test/java/security/DummyCredentialGenerator.java 86b26a7 > geode-core/src/test/java/security/LdapUserCredentialGenerator.java 12bcb62 > geode-core/src/test/java/security/PKCSCredentialGenerator.java 24c0100 > geode-core/src/test/java/security/SSLCredentialGenerator.java 29a1a30 > geode-core/src/test/java/security/UserPasswordWithExtraPropsAuthInit.java > a41f73a > geode-core/src/test/java/security/XmlAuthzCredentialGenerator.java 929eafb > geode-core/src/test/java/templates/security/DummyAuthenticator.java 5d33f22 > geode-core/src/test/java/templates/security/DummyAuthorization.java fe8e908 > geode-core/src/test/java/templates/security/FunctionSecurityPrmsHolder.java > 76827bb > geode-core/src/test/java/templates/security/LdapUserAuthenticator.java > db55219 > geode-core/src/test/java/templates/security/PKCSAuthInit.java d43b78e > geode-core/src/test/java/templates/security/PKCSAuthenticator.java d3610c4 > geode-core/src/test/java/templates/security/PKCSPrincipal.java 563689b > geode-core/src/test/java/templates/security/PKCSPrincipalTest.java > PRE-CREATION > geode-core/src/test/java/templates/security/UserPasswordAuthInit.java > f4b6eec > geode-core/src/test/java/templates/security/UsernamePrincipal.java 739dd52 > geode-core/src/test/java/templates/security/UsernamePrincipalTest.java > PRE-CREATION > geode-core/src/test/java/templates/security/XmlAuthorization.java 1ed0142 > geode-core/src/test/java/templates/security/XmlErrorHandler.java 5da8e09 > > geode-core/src/test/resources/com/gemstone/gemfire/codeAnalysis/sanctionedSerializables.txt > f3c1c5d > > Diff: https://reviews.apache.org/r/44402/diff/ > > > Testing > ------- > > precheckin > > > Thanks, > > Kirk Lund > >
