Skip to content

Commit

Permalink
[WrenSecurity#41] Fail Gracefully when Debug Provider Invalid
Browse files Browse the repository at this point in the history
- Ensure that debug provider fallback is in place BEFORE trying to log any exceptions about failing to initialize the debug provider.
- Upgrade `openam-shared` to JDK 8+ (see below).
- Clean-up debug provider initialization.
  - Switch to multi-catch instead of repetitive `catch` blocks (JDK 7+).
  - Pull-in `forgerock-guava-base` to allow us to use `Strings.isNullOrEmpty()`.
  - Switch to `Optional` to support chaining `trim()` on provider names without an explicit `null` check (JDK 8+).

Closes WrenSecurity#41.
  • Loading branch information
Kortanul committed Mar 2, 2019
1 parent 6056140 commit 842160c
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 25 deletions.
17 changes: 16 additions & 1 deletion openam-shared/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS HEADER.
*
* Copyright 2011-2016 ForgeRock AS.
* Portions Copyright 2018-2019 Wren Security.
*
* The contents of this file are subject to the terms
* of the Common Development and Distribution License
Expand Down Expand Up @@ -75,6 +76,15 @@
</archive>
</configuration>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-compiler-plugin</artifactId>

<configuration>
<source>8</source>
<target>8</target>
</configuration>
</plugin>
</plugins>
</build>

Expand All @@ -90,6 +100,7 @@
<artifactId>guice</artifactId>
<classifier>no_aop</classifier>
</dependency>

<dependency>
<groupId>com.google.inject.extensions</groupId>
<artifactId>guice-multibindings</artifactId>
Expand Down Expand Up @@ -146,6 +157,11 @@
<artifactId>forgerock-util</artifactId>
</dependency>

<dependency>
<groupId>org.forgerock.commons.guava</groupId>
<artifactId>forgerock-guava-base</artifactId>
</dependency>

<!-- IPv6 -->
<dependency>
<groupId>com.googlecode.java-ipv6</groupId>
Expand All @@ -161,6 +177,5 @@
<groupId>joda-time</groupId>
<artifactId>joda-time</artifactId>
</dependency>

</dependencies>
</project>
Original file line number Diff line number Diff line change
Expand Up @@ -25,22 +25,24 @@
* $Id: Debug.java,v 1.6 2009/08/19 05:41:17 veiming Exp $
*
* Portions Copyrighted 2013-2016 ForgeRock AS.
*
* Portions Copyright 2019 Wren Security.
*/

package com.sun.identity.shared.debug;

import java.util.Collection;
import java.util.HashMap;
import java.util.Map;
import java.util.Optional;
import java.util.ResourceBundle;

import org.slf4j.helpers.MessageFormatter;

import com.sun.identity.shared.configuration.SystemPropertiesManager;
import com.sun.identity.shared.debug.file.impl.StdDebugFile;
import com.sun.identity.shared.debug.impl.DebugProviderImpl;

import com.sun.identity.shared.locale.Locale;
import org.forgerock.guava.common.base.Strings;

// NOTE: Since JVM specs guarantee atomic access/updates to int variables
// (actually all variables except double and long), the design consciously
Expand Down Expand Up @@ -259,38 +261,45 @@ static IDebugProvider getDebugProvider() {
*/
private static synchronized void initialize() {
if (!serviceInitialized) {
String providerName = SystemPropertiesManager.get(DebugConstants.CONFIG_DEBUG_PROVIDER);
final String providerName;
IDebugProvider provider = null;
boolean providerLoadFailed = false;
Exception exceptionCatched = null;
if (providerName != null && providerName.trim().length() > 0) {
Exception exceptionCaught = null;

providerName =
Optional.ofNullable(
SystemPropertiesManager.get(DebugConstants.CONFIG_DEBUG_PROVIDER)
).map(String::trim)
.orElse(null);

if (!Strings.isNullOrEmpty(providerName)) {
try {
provider = (IDebugProvider) Class.forName(providerName).newInstance();
} catch (ClassNotFoundException cnex) {
provider = (IDebugProvider)Class.forName(providerName).newInstance();
} catch (ClassNotFoundException
| InstantiationException
| IllegalAccessException
| ClassCastException ex) {
providerLoadFailed = true;
exceptionCatched = cnex;
} catch (InstantiationException iex) {
providerLoadFailed = true;
exceptionCatched = iex;
} catch (IllegalAccessException iaex) {
providerLoadFailed = true;
exceptionCatched = iaex;
} catch (ClassCastException ccex) {
providerLoadFailed = true;
exceptionCatched = ccex;
exceptionCaught = ex;
}
}
if (provider == null) {
if (providerLoadFailed) {
ResourceBundle bundle = com.sun.identity.shared.locale.Locale.getInstallResourceBundle
("amUtilMsgs");
StdDebugFile.printError(Debug.class.getSimpleName(), bundle.getString("com.iplanet" + ".services" +
".debug.invalidprovider"), exceptionCatched);

}
if (provider == null) {
provider = new DebugProviderImpl();
}

setDebugProvider(provider);

if (providerLoadFailed) {
final ResourceBundle bundle = Locale.getInstallResourceBundle("amUtilMsgs");

StdDebugFile.printError(
Debug.class.getSimpleName(),
bundle.getString("com.iplanet.services.debug.invalidprovider"),
exceptionCaught
);
}

serviceInitialized = true;
}
}
Expand Down

0 comments on commit 842160c

Please sign in to comment.