[java-identity-provider] 01/02: Idp-installer more bnull checking
Rod Widdowson
rdw at steadingsoftware.com
Sun Jan 22 10:27:39 UTC 2023
This is an automated email from the git hooks/post-receive script.
rdw pushed a commit to branch main
in repository java-identity-provider.
View the commit online:
http://git.shibboleth.net/view/?p=java-identity-provider.git;a=commit;h=90bdc49143bfa8a1e499bb6335ef2fab963bdc9d
commit 90bdc49143bfa8a1e499bb6335ef2fab963bdc9d
Author: Rod Widdowson <rdw at steadingsoftware.com>
AuthorDate: Sat Jan 21 17:02:22 2023 +0000
Idp-installer more bnull checking
Notable, the plugin install must always have a non null httpClient
---
.../idp/installer/InstallerProperties.java | 1 +
.../idp/installer/PropertiesWithComments.java | 3 +-
.../net/shibboleth/idp/installer/V4Install.java | 5 ++-
.../idp/installer/plugin/impl/PluginInstaller.java | 47 ++++++++--------------
.../installer/plugin/impl/PluginInstallerCLI.java | 3 +-
.../installer/plugin/impl/PluginInstallerTest.java | 22 ++++++----
6 files changed, 38 insertions(+), 43 deletions(-)
diff --git a/idp-installer/src/main/java/net/shibboleth/idp/installer/InstallerProperties.java b/idp-installer/src/main/java/net/shibboleth/idp/installer/InstallerProperties.java
index c83f22c60..cbbdcba33 100644
--- a/idp-installer/src/main/java/net/shibboleth/idp/installer/InstallerProperties.java
+++ b/idp-installer/src/main/java/net/shibboleth/idp/installer/InstallerProperties.java
@@ -32,6 +32,7 @@ import net.shibboleth.shared.component.InitializableComponent;
public interface InstallerProperties extends InitializableComponent {
/** Those modules enabled by default. */
+ @SuppressWarnings("null")
@Nonnull public static final Set<String> DEFAULT_MODULES = Set.of("idp.authn.Password", "idp.admin.Hello");
/** Get where we are installing/updating/building the war.
diff --git a/idp-installer/src/main/java/net/shibboleth/idp/installer/PropertiesWithComments.java b/idp-installer/src/main/java/net/shibboleth/idp/installer/PropertiesWithComments.java
index 9abfb7cc2..bb4d4da20 100644
--- a/idp-installer/src/main/java/net/shibboleth/idp/installer/PropertiesWithComments.java
+++ b/idp-installer/src/main/java/net/shibboleth/idp/installer/PropertiesWithComments.java
@@ -26,7 +26,6 @@ import java.io.InputStreamReader;
import java.io.OutputStream;
import java.io.OutputStreamWriter;
import java.util.ArrayList;
-import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
@@ -78,7 +77,7 @@ public final class PropertiesWithComments {
* @param unreplacable names to warn on.
*/
public PropertiesWithComments(@Nonnull final Set<String> unreplacable) {
- unreplacableNames = Set.copyOf(unreplacable);
+ unreplacableNames = CollectionSupport.copyToSet(unreplacable);
}
/**
diff --git a/idp-installer/src/main/java/net/shibboleth/idp/installer/V4Install.java b/idp-installer/src/main/java/net/shibboleth/idp/installer/V4Install.java
index 493d69713..aa4cf6c16 100644
--- a/idp-installer/src/main/java/net/shibboleth/idp/installer/V4Install.java
+++ b/idp-installer/src/main/java/net/shibboleth/idp/installer/V4Install.java
@@ -243,12 +243,15 @@ public class V4Install extends AbstractInitializableComponent {
*/
// CheckStyle: CyclomaticComplexity|MethodLength OFF
protected void populatePropertyFiles(final boolean sealerCreated) throws BuildException {
- @Nonnull final Set<String> doNotReplaceList = Set.of(
+
+ final Set<String> dnrList = Set.of(
"idp.sealer.storePassword",
"idp.sealer.keyPassword",
"idp.authn.LDAP.bindDNCredential",
"idp.attribute.resolver.LDAP.bindDNCredential",
"idp.persistentId.salt");
+ assert dnrList != null;
+ @Nonnull final Set<String> doNotReplaceList =dnrList;
final Path conf = installerProps.getTargetDir().resolve("conf");
final Path dstConf = installerProps.getTargetDir().resolve("dist").resolve("conf");
diff --git a/idp-installer/src/main/java/net/shibboleth/idp/installer/plugin/impl/PluginInstaller.java b/idp-installer/src/main/java/net/shibboleth/idp/installer/plugin/impl/PluginInstaller.java
index 7004af228..ec33de50d 100644
--- a/idp-installer/src/main/java/net/shibboleth/idp/installer/plugin/impl/PluginInstaller.java
+++ b/idp-installer/src/main/java/net/shibboleth/idp/installer/plugin/impl/PluginInstaller.java
@@ -83,20 +83,18 @@ import net.shibboleth.shared.annotation.constraint.NotEmpty;
import net.shibboleth.shared.collection.CollectionSupport;
import net.shibboleth.shared.component.AbstractInitializableComponent;
import net.shibboleth.shared.component.ComponentInitializationException;
-import net.shibboleth.shared.httpclient.HttpClientBuilder;
import net.shibboleth.shared.logic.Constraint;
+import net.shibboleth.shared.logic.PredicateSupport;
import net.shibboleth.shared.primitive.StringSupport;
import net.shibboleth.shared.resource.Resource;
import net.shibboleth.shared.spring.httpclient.resource.HTTPResource;
-import net.shibboleth.shared.logic.PredicateSupport;
/**
* The class where the heavy lifting of managing a plugin happens.
*/
public final class PluginInstaller extends AbstractInitializableComponent implements AutoCloseable {
/** Class logger. */
- @Nonnull
- private static final Logger LOG = InstallationLogger.getLogger(PluginInstaller.class);
+ @Nonnull private static final Logger LOG = InstallationLogger.getLogger(PluginInstaller.class);
/** Property Name for version. */
private static final String PLUGIN_VERSION_PROPERTY ="idp.plugin.version";
@@ -132,7 +130,7 @@ public final class PluginInstaller extends AbstractInitializableComponent implem
private String truststore;
/** What to use to download things. */
- private HttpClient httpClient;
+ @Nonnull private final HttpClient httpClient;
/** If overridden these are the urls to us for update (rather than what the plugin asks for. */
@Nonnull private List<URL> updateOverrideURLs = CollectionSupport.emptyList();
@@ -173,6 +171,14 @@ public final class PluginInstaller extends AbstractInitializableComponent implem
/** Do we rebuild? */
private boolean rebuildWar = true;
+ /**
+ * Constructor.
+ * @param client - the HttpClient to use
+ */
+ public PluginInstaller(@Nonnull final HttpClient client) {
+ httpClient = client;
+ }
+
/** Set IdP Home.
* @param home Where we are working from
*/
@@ -202,13 +208,6 @@ public final class PluginInstaller extends AbstractInitializableComponent implem
acceptKey = Constraint.isNotNull(what, "Accept Key Predicate should be non-null");
}
- /** Set the httpClient.
- * @param what what to set.
- */
- public void setHttpClient(@Nonnull final HttpClient what) {
- httpClient = Constraint.isNotNull(what, "HttpClient should be non-null");
- }
-
/** Set the override URLS.
* @param urls The updateOverrideURLs to set.
*/
@@ -537,9 +536,11 @@ public final class PluginInstaller extends AbstractInitializableComponent implem
LOG.debug("{} not installed. files renamed", pluginId);
} else {
try {
+ final Path rollbackDir = workspacePath.resolve("rollback");
+ assert rollbackDir != null;
LOG.debug("Uninstalling version {} of {}", oldVersion, pluginId);
PluginInstallerSupport.renameToTree(pluginsWebapp,
- workspacePath.resolve("rollback"),
+ rollbackDir,
getInstalledContents(),
rollback.getFilesRenamedAway());
} catch (final IOException e) {
@@ -663,7 +664,6 @@ public final class PluginInstaller extends AbstractInitializableComponent implem
* @throws BuildException if badness is detected.
*/
private void download(@Nonnull final URL baseURL, @Nonnull final String fileName) throws BuildException {
- buildHttpClient();
try {
downloadDirectory = Files.createTempDirectory("plugin-installer-download");
final Resource baseResource = new HTTPResource(httpClient, baseURL);
@@ -675,21 +675,6 @@ public final class PluginInstaller extends AbstractInitializableComponent implem
}
}
- /** Build the Http Client if it doesn't exist. */
- private void buildHttpClient() {
- checkComponentActive();
- if (httpClient == null) {
- LOG.debug("No HttpClient built, creating default");
- try {
- httpClient = new HttpClientBuilder().buildClient();
- getModuleContext().setHttpClient(httpClient);
- } catch (final Exception e) {
- LOG.error("Could not create HttpClient", e);
- throw new BuildException(e);
- }
- }
- }
-
/** Capture module changes.
* @param changes what has changed */
private void captureChanges(final Map<ModuleResource,ResourceResult> changes) {
@@ -803,7 +788,9 @@ public final class PluginInstaller extends AbstractInitializableComponent implem
LOG.error("No contents unpacked from {}", fullName);
throw new BuildException("Distro was empty");
}
- distribution = PluginInstallerSupport.canonicalPath(contents.next());
+ final Path next = contents.next();
+ assert next != null;
+ distribution = PluginInstallerSupport.canonicalPath(next);
if (contents.hasNext()) {
LOG.error("Too many packages in distributions {}", fullName);
throw new BuildException("Too many packages in distributions");
diff --git a/idp-installer/src/main/java/net/shibboleth/idp/installer/plugin/impl/PluginInstallerCLI.java b/idp-installer/src/main/java/net/shibboleth/idp/installer/plugin/impl/PluginInstallerCLI.java
index 7d857a403..ed363ebcb 100644
--- a/idp-installer/src/main/java/net/shibboleth/idp/installer/plugin/impl/PluginInstallerCLI.java
+++ b/idp-installer/src/main/java/net/shibboleth/idp/installer/plugin/impl/PluginInstallerCLI.java
@@ -140,7 +140,7 @@ public final class PluginInstallerCLI extends AbstractIdPHomeAwareCommandLine<Pl
updateURLs = Collections.emptyList();
}
- try (final PluginInstaller inst = new PluginInstaller()){
+ try (final PluginInstaller inst = new PluginInstaller(Constraint.isNotNull(getHttpClient(), "HJttpClient cannot be non null (by construction"))) {
constructPluginInstaller(inst, args);
assert inst == installer;
final String pluginId = args.getPluginId();
@@ -228,7 +228,6 @@ public final class PluginInstallerCLI extends AbstractIdPHomeAwareCommandLine<Pl
// This is null because we set up the bean name before calling super.dorun
//
assert client != null;
- inst.setHttpClient(client);
inst.setModuleContextSecurityParams(getHttpClientSecurityParameters());
assert(updateURLs != null);
inst.setUpdateOverrideURLs(updateURLs);
diff --git a/idp-installer/src/test/java/net/shibboleth/idp/installer/plugin/impl/PluginInstallerTest.java b/idp-installer/src/test/java/net/shibboleth/idp/installer/plugin/impl/PluginInstallerTest.java
index 6a5add107..04345a596 100644
--- a/idp-installer/src/test/java/net/shibboleth/idp/installer/plugin/impl/PluginInstallerTest.java
+++ b/idp-installer/src/test/java/net/shibboleth/idp/installer/plugin/impl/PluginInstallerTest.java
@@ -31,6 +31,7 @@ import java.util.stream.Collectors;
import javax.annotation.Nonnull;
+import org.apache.http.client.HttpClient;
import org.bouncycastle.jce.provider.BouncyCastleProvider;
import org.slf4j.Logger;
import org.testng.annotations.BeforeClass;
@@ -39,13 +40,16 @@ import org.testng.annotations.Test;
import net.shibboleth.idp.plugin.AbstractIdPPlugin;
import net.shibboleth.idp.plugin.IdPPlugin;
import net.shibboleth.shared.component.ComponentInitializationException;
+import net.shibboleth.shared.httpclient.HttpClientBuilder;
import net.shibboleth.shared.primitive.LoggerFactory;
- at SuppressWarnings("javadoc")
+ at SuppressWarnings({"javadoc", "null"})
public class PluginInstallerTest extends BasePluginTest {
private final Logger log = LoggerFactory.getLogger(PluginInstallerTest.class);
-
+
+ private HttpClient client;
+
private final Predicate<String> loggingAcceptCert = new Predicate<>() {
public boolean test(String what) {
log.debug("Accepting the certificate\n{}", what);
@@ -53,15 +57,17 @@ public class PluginInstallerTest extends BasePluginTest {
}
};
- @BeforeClass public void setup() throws IOException {
+
+ @BeforeClass public void setup() throws Exception {
if (Security.getProvider(BouncyCastleProvider.PROVIDER_NAME) == null) {
Security.addProvider(new BouncyCastleProvider());
}
+ client = new HttpClientBuilder().buildClient();
}
@Test(enabled = false) public void testListing() throws ComponentInitializationException, IOException {
- try (final PluginInstaller inst = new PluginInstaller()) {
+ try (final PluginInstaller inst = new PluginInstaller(client)) {
inst.setIdpHome(getIdpHome());
inst.initialize();
final Map<String, Object> result = inst.getInstalledPlugins().stream().collect(Collectors.toMap(IdPPlugin::getPluginId,
@@ -74,7 +80,7 @@ public class PluginInstallerTest extends BasePluginTest {
@Test(enabled = false, dependsOnMethods ={"testListing", }) public void testRemove() throws ComponentInitializationException, IOException
{
- try (final PluginInstaller inst = new PluginInstaller()) {
+ try (final PluginInstaller inst = new PluginInstaller(client)) {
inst.setIdpHome(getIdpHome());
inst.setPluginId("org.example.Plugin");
inst.initialize();
@@ -83,7 +89,7 @@ public class PluginInstallerTest extends BasePluginTest {
}
@Test(enabled = false) public void testUnpackZip() throws ComponentInitializationException, IOException {
- try (final PluginInstaller inst = new PluginInstaller()) {
+ try (final PluginInstaller inst = new PluginInstaller(client)) {
inst.setIdpHome(getIdpHome());
inst.setAcceptKey(loggingAcceptCert);
inst.initialize();
@@ -94,7 +100,7 @@ public class PluginInstallerTest extends BasePluginTest {
}
@Test(enabled = false) public void testUnpackZipFile() throws ComponentInitializationException, IOException {
- try (final PluginInstaller inst = new PluginInstaller()) {
+ try (final PluginInstaller inst = new PluginInstaller(client)) {
inst.setIdpHome(getIdpHome());
inst.setAcceptKey(loggingAcceptCert);
inst.initialize();
@@ -105,7 +111,7 @@ public class PluginInstallerTest extends BasePluginTest {
@Test(enabled = false) public void testUnpackTgz() throws ComponentInitializationException, IOException {
- try (final PluginInstaller inst = new PluginInstaller()) {
+ try (final PluginInstaller inst = new PluginInstaller(client)) {
inst.setPluginId("net.shibboleth.idp.plugin.rhino");
inst.setIdpHome(getIdpHome());
inst.setAcceptKey(loggingAcceptCert);
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list