[ca-certificates-java] 39/46: Extracted the keystore handling code from UpdateCertificates into a distinct class

Emmanuel Bourg ebourg-guest at moszumanska.debian.org
Mon Feb 2 21:25:32 UTC 2015


This is an automated email from the git hooks/post-receive script.

ebourg-guest pushed a commit to branch master
in repository ca-certificates-java.

commit 42bedee6326199845b1e127176552185fc5a8521
Author: Emmanuel Bourg <ebourg at apache.org>
Date:   Mon Mar 24 12:32:19 2014 +0000

    Extracted the keystore handling code from UpdateCertificates into a distinct class
---
 debian/changelog                                   |   1 +
 .../java/org/debian/security/KeyStoreHandler.java  | 146 +++++++++++++++++++++
 .../org/debian/security/UpdateCertificates.java    | 133 ++-----------------
 .../org/debian/security/KeyStoreHandlerTest.java   |  90 +++++++++++++
 .../debian/security/UpdateCertificatesTest.java    | 107 ++++-----------
 5 files changed, 278 insertions(+), 199 deletions(-)

diff --git a/debian/changelog b/debian/changelog
index 36586ec..fb1d71a 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -6,6 +6,7 @@ ca-certificates-java (20130816) UNRELEASED; urgency=medium
   * Limit the memory used by java to 64M when updating the certificates
     (Closes: 576453)
   * Mavenized the project
+  * Code refactoring
   * d/control: Standards-Version updated to 3.9.5 (no changes)
   * Switch to debhelper level 9
 
diff --git a/src/main/java/org/debian/security/KeyStoreHandler.java b/src/main/java/org/debian/security/KeyStoreHandler.java
new file mode 100644
index 0000000..6ed7c72
--- /dev/null
+++ b/src/main/java/org/debian/security/KeyStoreHandler.java
@@ -0,0 +1,146 @@
+/*
+ * Copyright (C) 2011 Torsten Werner <twerner at debian.org>
+ * Copyright (C) 2012 Damien Raude-Morvan <drazzib at debian.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+package org.debian.security;
+
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.security.GeneralSecurityException;
+import java.security.KeyStore;
+import java.security.KeyStoreException;
+import java.security.cert.Certificate;
+import java.security.cert.CertificateFactory;
+
+/**
+ * Handles read/write operations on a keystore.
+ */
+class KeyStoreHandler {
+
+    /** The path of the keystore */
+    private String filename;
+
+    /** The password of the keystore */
+    private char[] password;
+
+    private KeyStore ks;
+    
+    private CertificateFactory certFactory;
+
+    KeyStoreHandler(String filename, char[] password) throws GeneralSecurityException, IOException, InvalidKeystorePasswordException {
+        this.filename = filename;
+        this.password = password;
+        this.certFactory = CertificateFactory.getInstance("X.509");
+        
+        load();
+    }
+
+    /**
+     * Try to open an existing keystore or create an new one.
+     */
+    public void load() throws GeneralSecurityException, IOException, InvalidKeystorePasswordException {
+        KeyStore ks = KeyStore.getInstance(KeyStore.getDefaultType());
+        File file = new File(filename);
+        FileInputStream in = null;
+        if (file.canRead()) {
+            in = new FileInputStream(file);
+        }
+        try {
+            ks.load(in, password);
+        } catch (IOException e) {
+            throw new InvalidKeystorePasswordException("Cannot open Java keystore. Is the password correct?", e);
+        } finally {
+            if (in != null) {
+                in.close();
+            }
+        }
+        this.ks = ks;
+    }
+
+    /**
+     * Write actual keystore content to disk.
+     */
+    public void save() throws GeneralSecurityException, UnableToSaveKeystoreException {
+        try {
+            FileOutputStream certOutputFile = new FileOutputStream(filename);
+            ks.store(certOutputFile, password);
+            certOutputFile.close();
+        } catch (IOException e) {
+            throw new UnableToSaveKeystoreException("There was a problem saving the new Java keystore.", e);
+        }
+    }
+
+    /**
+     * Add or replace existing cert in keystore with given alias.
+     */
+    public void addAlias(String alias, String path) throws KeyStoreException {
+        Certificate cert = loadCertificate(path);
+        if (cert == null) {
+            return;
+        }
+        addAlias(alias, cert);
+    }
+    
+    /**
+     * Add or replace existing cert in keystore with given alias.
+     */
+    public void addAlias(String alias, Certificate cert) throws KeyStoreException {
+        if (contains(alias)) {
+            System.out.println("Replacing " + alias);
+            ks.deleteEntry(alias);
+        } else {
+            System.out.println("Adding " + alias);
+        }
+        ks.setCertificateEntry(alias, cert);
+    }
+
+    /**
+     * Delete cert in keystore at given alias.
+     */
+    public void deleteAlias(String alias) throws GeneralSecurityException {
+        if (contains(alias)) {
+            System.out.println("Removing " + alias);
+            ks.deleteEntry(alias);
+        }
+    }
+
+    /**
+     * Returns true when alias exist in keystore.
+     */
+    public boolean contains(String alias) throws KeyStoreException {
+        return ks.containsAlias(alias);
+    }
+
+    /**
+     * Try to load a certificate instance from given path.
+     */
+    private Certificate loadCertificate(String path) {
+        Certificate certificate = null;
+        try {
+            FileInputStream in = new FileInputStream(path);
+            certificate = certFactory.generateCertificate(in);
+            in.close();
+        } catch (Exception e) {
+            System.err.println("Warning: there was a problem reading the certificate file " +
+                    path + ". Message:\n  " + e.getMessage());
+        }
+        return certificate;
+    }
+}
diff --git a/src/main/java/org/debian/security/UpdateCertificates.java b/src/main/java/org/debian/security/UpdateCertificates.java
index 495f37e..e4f8205 100644
--- a/src/main/java/org/debian/security/UpdateCertificates.java
+++ b/src/main/java/org/debian/security/UpdateCertificates.java
@@ -15,23 +15,16 @@
  * You should have received a copy of the GNU General Public License along
  * with this program; if not, write to the Free Software Foundation, Inc.,
  * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
- *
  */
 
 package org.debian.security;
 
 import java.io.BufferedReader;
-import java.io.File;
-import java.io.FileInputStream;
-import java.io.FileOutputStream;
 import java.io.IOException;
 import java.io.InputStreamReader;
 import java.io.Reader;
 import java.security.GeneralSecurityException;
-import java.security.KeyStore;
-import java.security.KeyStoreException;
 import java.security.cert.Certificate;
-import java.security.cert.CertificateFactory;
 
 /**
  * This code is a re-implementation of the idea from Ludwig Nussel found in
@@ -43,13 +36,7 @@ import java.security.cert.CertificateFactory;
  */
 public class UpdateCertificates {
 
-    private char[] password = null;
-
-    private String ksFilename = null;
-
-    private KeyStore ks = null;
-
-    private CertificateFactory certFactory = null;
+    private KeyStoreHandler keystore;
 
     public static void main(String[] args) throws IOException, GeneralSecurityException {
         String passwordString = "changeit";
@@ -61,10 +48,10 @@ public class UpdateCertificates {
         }
 
         try {
-            UpdateCertificates uc = new UpdateCertificates(passwordString, "/etc/ssl/certs/java/cacerts");
+            UpdateCertificates uc = new UpdateCertificates("/etc/ssl/certs/java/cacerts", passwordString);
             // Force reading of inputstream in UTF-8
             uc.processChanges(new InputStreamReader(System.in, "UTF8"));
-            uc.writeKeyStore();
+            uc.finish();
         } catch (InvalidKeystorePasswordException e) {
             e.printStackTrace(System.err);
             System.exit(1);
@@ -74,41 +61,17 @@ public class UpdateCertificates {
         }
     }
 
-    public UpdateCertificates(final String passwordString, final String keystoreFile) throws IOException, GeneralSecurityException, InvalidKeystorePasswordException {
-        this.password = passwordString.toCharArray();
-        this.ksFilename = keystoreFile;
-        this.ks = openKeyStore();
-        this.certFactory = CertificateFactory.getInstance("X.509");
-    }
-
-    /**
-     * Try to open a existing keystore or create an new one.
-     */
-    private KeyStore openKeyStore() throws GeneralSecurityException, IOException, InvalidKeystorePasswordException {
-        KeyStore ks = KeyStore.getInstance(KeyStore.getDefaultType());
-        File certInputFile = new File(this.ksFilename);
-        FileInputStream certInputStream = null;
-        if (certInputFile.canRead()) {
-            certInputStream = new FileInputStream(certInputFile);
-        }
-        try {
-            ks.load(certInputStream, this.password);
-        } catch (IOException e) {
-            throw new InvalidKeystorePasswordException("Cannot open Java keystore. Is the password correct?", e);
-        }
-        if (certInputStream != null) {
-            certInputStream.close();
-        }
-        return ks;
+    public UpdateCertificates(String keystoreFile, String password) throws IOException, GeneralSecurityException, InvalidKeystorePasswordException {
+        this.keystore = new KeyStoreHandler(keystoreFile, password.toCharArray());
     }
 
     /**
      * Until reader EOF, try to read changes and send each to {@link #parseLine(String)}.
      */
-    protected void processChanges(final Reader reader) throws IOException, GeneralSecurityException {
+    protected void processChanges(Reader reader) throws IOException, GeneralSecurityException {
         String line;
-        BufferedReader bufferedStdinReader = new BufferedReader(reader);
-        while ((line = bufferedStdinReader.readLine()) != null) {
+        BufferedReader in = new BufferedReader(reader);
+        while ((line = in.readLine()) != null) {
             try {
                 parseLine(line);
             } catch (UnknownInputException e) {
@@ -123,93 +86,25 @@ public class UpdateCertificates {
      * or {@link #deleteAlias(String)}.
      */
     protected void parseLine(final String line) throws GeneralSecurityException, IOException, UnknownInputException {
-        assert this.ks != null;
-
         String path = line.substring(1);
         String filename = path.substring(path.lastIndexOf("/") + 1);
         String alias = "debian:" + filename;
         if (line.startsWith("+")) {
-            Certificate cert = loadCertificate(path);
-            if (cert == null) {
-                return;
-            }
-            addAlias(alias, cert);
+            keystore.addAlias(alias, path);
         } else if (line.startsWith("-")) {
-            deleteAlias(alias);
+            keystore.deleteAlias(alias);
             // Remove old non-prefixed aliases, too. This code should be
             // removed after the release of Wheezy.
-            deleteAlias(filename);
+            keystore.deleteAlias(filename);
         } else {
             throw new UnknownInputException(line);
         }
     }
 
     /**
-     * Delete cert in keystore at given alias.
-     */
-    private void deleteAlias(final String alias) throws GeneralSecurityException {
-        assert this.ks != null;
-
-        if (contains(alias)) {
-            System.out.println("Removing " + alias);
-            this.ks.deleteEntry(alias);
-        }
-    }
-
-    /**
-     * Add or replace existing cert in keystore with given alias.
-     */
-    private void addAlias(final String alias, final Certificate cert) throws KeyStoreException {
-        assert this.ks != null;
-
-        if (contains(alias)) {
-            System.out.println("Replacing " + alias);
-            this.ks.deleteEntry(alias);
-        } else {
-            System.out.println("Adding " + alias);
-        }
-        this.ks.setCertificateEntry(alias, cert);
-    }
-
-    /**
-     * Returns true when alias exist in keystore.
-     */
-    protected boolean contains(String alias) throws KeyStoreException {
-        assert this.ks != null;
-
-        return this.ks.containsAlias(alias);
-    }
-
-    /**
-     * Try to load a certificate instance from given path.
+     * Write the pending changes to the keystore file.
      */
-    private Certificate loadCertificate(final String path) {
-        assert this.certFactory != null;
-
-        Certificate cert = null;
-        try {
-            FileInputStream certFile = new FileInputStream(path);
-            cert = this.certFactory.generateCertificate(certFile);
-            certFile.close();
-        } catch (Exception e) {
-            System.err.println("Warning: there was a problem reading the certificate file " +
-                    path + ". Message:\n  " + e.getMessage());
-        }
-        return cert;
-    }
-
-    /**
-     * Write actual keystore content to disk.
-     */
-    protected void writeKeyStore() throws GeneralSecurityException, UnableToSaveKeystoreException {
-        assert this.ks != null;
-
-        try {
-            FileOutputStream certOutputFile = new FileOutputStream(this.ksFilename);
-            this.ks.store(certOutputFile, this.password);
-            certOutputFile.close();
-        } catch (IOException e) {
-            throw new UnableToSaveKeystoreException("There was a problem saving the new Java keystore.", e);
-        }
+    protected void finish() throws GeneralSecurityException, UnableToSaveKeystoreException {
+        keystore.save();
     }
 }
diff --git a/src/test/java/org/debian/security/KeyStoreHandlerTest.java b/src/test/java/org/debian/security/KeyStoreHandlerTest.java
new file mode 100644
index 0000000..964dc50
--- /dev/null
+++ b/src/test/java/org/debian/security/KeyStoreHandlerTest.java
@@ -0,0 +1,90 @@
+/*
+ * Copyright (C) 2012 Damien Raude-Morvan <drazzib at debian.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+package org.debian.security;
+
+import java.io.File;
+
+import org.junit.Test;
+
+import static org.junit.Assert.*;
+
+/**
+ * @author Emmanuel Bourg
+ * @version $Revision$, $Date$
+ */
+public class KeyStoreHandlerTest {
+
+    private String ksFilename = "./target/test-classes/tests-cacerts";
+    private char[] ksPassword = "changeit".toCharArray();
+
+    /**
+     * Test a simple open then write without any modification.
+     */
+    @Test
+    public void testNoop() throws Exception {
+        KeyStoreHandler keystore = new KeyStoreHandler(ksFilename, ksPassword);
+        keystore.save();
+    }
+
+    /**
+     * Test a to open a keystore and write without any modification
+     * and then try to open it again with wrong password : will throw a
+     * InvalidKeystorePassword
+     */
+    @Test
+    public void testWriteThenOpenWrongPwd() throws Exception {
+        try {
+            KeyStoreHandler keystore = new KeyStoreHandler(ksFilename, ksPassword);
+            keystore.save();
+        } catch (InvalidKeystorePasswordException e) {
+            fail();
+        }
+
+        try {
+            KeyStoreHandler keystore = new KeyStoreHandler(ksFilename, "wrongpassword".toCharArray());
+            fail();
+            keystore.save();
+        } catch (InvalidKeystorePasswordException e) {
+            assertEquals("Cannot open Java keystore. Is the password correct?", e.getMessage());
+        }
+    }
+
+    /**
+     * Test a to open a keystore then remove its backing File (and replace it
+     * with a directory with the same name) and try to write in to disk :
+     * will throw an UnableToSaveKeystore
+     */
+    @Test
+    public void testDeleteThenWrite() throws Exception {
+        try {
+            KeyStoreHandler keystore = new KeyStoreHandler(ksFilename, ksPassword);
+
+            // Replace actual file by a directory !
+            File file = new File(ksFilename);
+            file.delete();
+            file.mkdir();
+
+            // Will fail with some IOException
+            keystore.save();
+            fail();
+        } catch (UnableToSaveKeystoreException e) {
+            assertEquals("There was a problem saving the new Java keystore.", e.getMessage());
+        }
+    }
+}
diff --git a/src/test/java/org/debian/security/UpdateCertificatesTest.java b/src/test/java/org/debian/security/UpdateCertificatesTest.java
index 5e5ed27..ea68239 100644
--- a/src/test/java/org/debian/security/UpdateCertificatesTest.java
+++ b/src/test/java/org/debian/security/UpdateCertificatesTest.java
@@ -14,7 +14,6 @@
  * You should have received a copy of the GNU General Public License along
  * with this program; if not, write to the Free Software Foundation, Inc.,
  * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
- *
  */
 
 package org.debian.security;
@@ -38,79 +37,22 @@ public class UpdateCertificatesTest {
     private static final String REMOVE_CACERT  = "-/usr/share/ca-certificates/spi-inc.org/spi-cacert-2008.crt";
     private static final String ADD_CACERT     = "+/usr/share/ca-certificates/spi-inc.org/spi-cacert-2008.crt";
 
-    private String ksFilename;
-    private String ksPassword;
+    private String ksFilename = "./target/test-classes/tests-cacerts";
+    private String ksPassword = "changeit";
 
     @Before
     public void start() {
-        ksFilename = "./target/test-classes/tests-cacerts";
-        ksPassword = "changeit";
         // Delete any previous file
         File keystore = new File(ksFilename);
         keystore.delete();
     }
 
     /**
-     * Test a simple open then write without any modification.
-     */
-    @Test
-    public void testNoop() throws Exception {
-        UpdateCertificates uc = new UpdateCertificates(ksPassword, ksFilename);
-        uc.writeKeyStore();
-    }
-
-    /**
-     * Test a to open a keystore and write without any modification
-     * and then try to open it again with wrong password : will throw a
-     * InvalidKeystorePassword
-     */
-    @Test
-    public void testWriteThenOpenWrongPwd() throws Exception {
-        try {
-            UpdateCertificates uc = new UpdateCertificates(ksPassword, ksFilename);
-            uc.writeKeyStore();
-        } catch (InvalidKeystorePasswordException e) {
-            fail();
-        }
-
-        try {
-            UpdateCertificates uc = new UpdateCertificates("wrongpassword", ksFilename);
-            fail();
-            uc.writeKeyStore();
-        } catch (InvalidKeystorePasswordException e) {
-            assertEquals("Cannot open Java keystore. Is the password correct?", e.getMessage());
-        }
-    }
-
-    /**
-     * Test a to open a keystore then remove its backing File (and replace it
-     * with a directory with the same name) and try to write in to disk :
-     * will throw an UnableToSaveKeystore
-     */
-    @Test
-    public void testDeleteThenWrite() throws Exception {
-        try {
-            UpdateCertificates uc = new UpdateCertificates(ksPassword, ksFilename);
-
-            // Replace actual file by a directory !
-            File keystore = new File(ksFilename);
-            keystore.delete();
-            keystore.mkdir();
-
-            // Will fail with some IOException
-            uc.writeKeyStore();
-            fail();
-        } catch (UnableToSaveKeystoreException e) {
-            assertEquals("There was a problem saving the new Java keystore.", e.getMessage());
-        }
-    }
-
-    /**
      * Try to send an invalid command ("x") in parseLine : throw UnknownInput
      */
     @Test
     public void testWrongCommand() throws Exception {
-        UpdateCertificates uc = new UpdateCertificates(ksPassword, ksFilename);
+        UpdateCertificates uc = new UpdateCertificates(ksFilename, ksPassword);
         try {
             uc.parseLine(INVALID_CACERT);
             fail();
@@ -124,11 +66,12 @@ public class UpdateCertificatesTest {
      */
     @Test
     public void testAdd() throws Exception {
-        UpdateCertificates uc = new UpdateCertificates(ksPassword, ksFilename);
+        UpdateCertificates uc = new UpdateCertificates(ksFilename, ksPassword);
         uc.parseLine(ADD_CACERT);
-        uc.writeKeyStore();
+        uc.finish();
 
-        assertEquals(true, uc.contains(ALIAS_CACERT));
+        KeyStoreHandler keystore = new KeyStoreHandler(ksFilename, ksPassword.toCharArray());
+        assertEquals(true, keystore.contains(ALIAS_CACERT));
     }
 
     /**
@@ -137,11 +80,12 @@ public class UpdateCertificatesTest {
      */
     @Test
     public void testAddInvalidCert() throws Exception {
-        UpdateCertificates uc = new UpdateCertificates(ksPassword, ksFilename);
+        UpdateCertificates uc = new UpdateCertificates(ksFilename, ksPassword);
         uc.parseLine("+/usr/share/ca-certificates/null.crt");
-        uc.writeKeyStore();
+        uc.finish();
 
-        assertEquals(false, uc.contains("debian:null.crt"));
+        KeyStoreHandler keystore = new KeyStoreHandler(ksFilename, ksPassword.toCharArray());
+        assertEquals(false, keystore.contains("debian:null.crt"));
     }
 
     /**
@@ -150,12 +94,13 @@ public class UpdateCertificatesTest {
      */
     @Test
     public void testReplace() throws Exception {
-        UpdateCertificates uc = new UpdateCertificates(ksPassword, ksFilename);
+        UpdateCertificates uc = new UpdateCertificates(ksFilename, ksPassword);
         uc.parseLine(ADD_CACERT);
         uc.parseLine(ADD_CACERT);
-        uc.writeKeyStore();
+        uc.finish();
 
-        assertEquals(true, uc.contains(ALIAS_CACERT));
+        KeyStoreHandler keystore = new KeyStoreHandler(ksFilename, ksPassword.toCharArray());
+        assertEquals(true, keystore.contains(ALIAS_CACERT));
     }
 
     /**
@@ -163,12 +108,13 @@ public class UpdateCertificatesTest {
      */
     @Test
     public void testRemove() throws Exception {
-        UpdateCertificates uc = new UpdateCertificates(ksPassword, ksFilename);
+        UpdateCertificates uc = new UpdateCertificates(ksFilename, ksPassword);
         uc.parseLine(REMOVE_CACERT);
-        uc.writeKeyStore();
+        uc.finish();
 
         // We start with empty KS, so it shouldn't do anything
-        assertEquals(false, uc.contains(ALIAS_CACERT));
+        KeyStoreHandler keystore = new KeyStoreHandler(ksFilename, ksPassword.toCharArray());
+        assertEquals(false, keystore.contains(ALIAS_CACERT));
     }
 
     /**
@@ -176,17 +122,18 @@ public class UpdateCertificatesTest {
      */
     @Test
     public void testAddThenRemove() throws Exception {
-        UpdateCertificates ucAdd = new UpdateCertificates(ksPassword, ksFilename);
+        UpdateCertificates ucAdd = new UpdateCertificates(ksFilename, ksPassword);
         ucAdd.parseLine(ADD_CACERT);
-        ucAdd.writeKeyStore();
+        ucAdd.finish();
 
-        assertEquals(true, ucAdd.contains(ALIAS_CACERT));
+        KeyStoreHandler keystore = new KeyStoreHandler(ksFilename, ksPassword.toCharArray());
+        assertEquals(true, keystore.contains(ALIAS_CACERT));
 
-        UpdateCertificates ucRemove = new UpdateCertificates(ksPassword, ksFilename);
+        UpdateCertificates ucRemove = new UpdateCertificates(ksFilename, ksPassword);
         ucRemove.parseLine(REMOVE_CACERT);
-        ucRemove.writeKeyStore();
+        ucRemove.finish();
 
-        assertEquals(false, ucRemove.contains(ALIAS_CACERT));
+        keystore.load();
+        assertEquals(false, keystore.contains(ALIAS_CACERT));
     }
-
 }

-- 
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-java/ca-certificates-java.git



More information about the pkg-java-commits mailing list