[prev in list] [next in list] [prev in thread] [next in thread]
List: bouncycastle-crypto-dev
Subject: [dev-crypto] Tiny refactoring of PGPKeyRing
From: Shawn Willden <swillden () google ! com>
Date: 2011-06-22 20:54:09
Message-ID: BANLkTimntxC_ob5tkk+igGFzURkp_BbEraD9-LHa9kgK7H5Xag () mail ! gmail ! com
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
Hi,
In some work I've been doing with BCPGP, I've found it annoying in many
cases that there are various closely-related classes with common
superclasses and common methods... but the common methods aren't in the
superclasses. This leads to a lot of code like:
Baseclass obj = //...
if (obj instanceof SubclassOne) {
obj.commonMethod(foo);
} else if (obj.instanceof SubclassTwo) {
obj.commonMethod(foo);
}
Simply pulling commonMethod() up to the common base class as an abstract
method would obviously reduce the above to:
Baseclass obj = //...
obj.commonMethod(foo);
which, assuming the semantics of commonMethod() are sensible, is much
cleaner and clearer.
So, I've decided to give back a bit by providing some of these trivial
refactorings, to help make BCPGP a little more user-friendly. In this first
one, I just make the PGPKeyRing class useful by adding public abstract
methods corresponding to the common methods in PGPPublicKeyRing and
PGPSecretKeyRing. In two cases, PGPSecretKeyRing didn't actually have the
method but implementation was straightforward.
I didn't create real unit tests because the code is so trivial. It's
possible that PGPSecretKeyRing.getPublicKeys() should have a unit test. I
did modify some of the existing tests to use the PGPKeyRing interface, just
to prove that the results are the same.
Another set of classes that has this problem is the PGPSignature classes.
I'll probably get them next.
--
Shawn
[Attachment #5 (text/html)]
Hi,<div><br></div><div>In some work I've been doing with BCPGP, I've found it \
annoying in many cases that there are various closely-related classes with common \
superclasses and common methods... but the common methods aren't in the \
superclasses. This leads to a lot of code like:</div> <div><br></div><div>Baseclass \
obj = //...</div><div>if (obj instanceof SubclassOne) {</div><div> \
obj.commonMethod(foo);</div><div>} else if (obj.instanceof SubclassTwo) {</div><div> \
obj.commonMethod(foo);</div><div> }</div><div><br></div><div>Simply pulling \
commonMethod() up to the common base class as an abstract method would obviously \
reduce the above to:</div><div><br></div><div>Baseclass obj = \
//...</div><div>obj.commonMethod(foo);</div> <div><br></div><div>which, assuming the \
semantics of commonMethod() are sensible, is much cleaner and \
clearer.</div><div><br></div><div>So, I've decided to give back a bit by \
providing some of these trivial refactorings, to help make BCPGP a little more \
user-friendly. In this first one, I just make the PGPKeyRing class useful by adding \
public abstract methods corresponding to the common methods in PGPPublicKeyRing and \
PGPSecretKeyRing. In two cases, PGPSecretKeyRing didn't actually have the method \
but implementation was straightforward.</div> <div><br></div><div>I didn't create \
real unit tests because the code is so trivial. It's possible that \
PGPSecretKeyRing.getPublicKeys() should have a unit test. I did modify some of the \
existing tests to use the PGPKeyRing interface, just to prove that the results are \
the same.</div> <div><br></div><div>Another set of classes that has this problem is \
the PGPSignature classes. I'll probably get them \
next.</div><div><br></div><div>-- </div><div>Shawn</div>
--001636c927bc81b11004a6532b7f--
["bcpgp_keyring_refactor.patch" (text/x-patch)]
diff --git a/google3/third_party/java/bouncycastle_bcpg/v1_46/bcpg-jdk15-146/src/org/bouncycastle/openpgp/PGPKeyRing.java \
b/google3/third_party/java/bouncycastle_bcpg/v1_46/bcpg-jdk15-146/src/org/bouncycastle/openpgp/PGPKeyRing.java
index f518faf..640780c 100644
--- a/google3/third_party/java/bouncycastle_bcpg/v1_46/bcpg-jdk15-146/src/org/bouncycastle/openpgp/PGPKeyRing.java
+++ b/google3/third_party/java/bouncycastle_bcpg/v1_46/bcpg-jdk15-146/src/org/bouncycastle/openpgp/PGPKeyRing.java
@@ -10,7 +10,9 @@ import org.bouncycastle.bcpg.UserIDPacket;
import java.io.IOException;
import java.io.InputStream;
+import java.io.OutputStream;
import java.util.ArrayList;
+import java.util.Iterator;
import java.util.List;
public abstract class PGPKeyRing
@@ -89,4 +91,32 @@ public abstract class PGPKeyRing
idSigs.add(readSignaturesAndTrust(pIn));
}
}
+
+ public abstract void encode(OutputStream outStream) throws IOException;
+
+ public abstract byte[] getEncoded() throws IOException;
+
+ /**
+ * Return the first public key in the ring. In the case of a {@link \
PGPSecretKeyRing} + * this is also the public key of the master key pair.
+ *
+ * @return PGPPublicKey
+ */
+ public abstract PGPPublicKey getPublicKey();
+
+ /**
+ * Return an iterator containing all the public keys.
+ *
+ * @return Iterator
+ */
+ public abstract Iterator getPublicKeys();
+
+ /**
+ * Return the public key referred to by the passed in keyID if it
+ * is present.
+ *
+ * @param keyID
+ * @return PGPPublicKey
+ */
+ public abstract PGPPublicKey getPublicKey(long keyID);
}
diff --git a/google3/third_party/java/bouncycastle_bcpg/v1_46/bcpg-jdk15-146/src/org/bouncycastle/openpgp/PGPSecretKeyRing.java \
b/google3/third_party/java/bouncycastle_bcpg/v1_46/bcpg-jdk15-146/src/org/bouncycastle/openpgp/PGPSecretKeyRing.java
index 3ac105b..41bccb6 100644
--- a/google3/third_party/java/bouncycastle_bcpg/v1_46/bcpg-jdk15-146/src/org/bouncycastle/openpgp/PGPSecretKeyRing.java
+++ b/google3/third_party/java/bouncycastle_bcpg/v1_46/bcpg-jdk15-146/src/org/bouncycastle/openpgp/PGPSecretKeyRing.java
@@ -210,6 +210,56 @@ public class PGPSecretKeyRing
}
/**
+ * Return an iterator over all public keys in the keyring, including both the \
public + * keys corresponding to secret keys returned by {@link #getSecretKeys()} \
and the other + * public keys returned by {@link #getExtraPublicKeys()}.
+ */
+ public Iterator<PGPPublicKey> getPublicKeys()
+ {
+ final Iterator secretKeys = getSecretKeys();
+ final Iterator extraKeys = getExtraPublicKeys();
+ return new Iterator()
+ {
+ public boolean hasNext()
+ {
+ return secretKeys.hasNext() || extraKeys.hasNext();
+ }
+ public Object next()
+ {
+ if (secretKeys.hasNext())
+ {
+ return ((PGPSecretKey) secretKeys.next()).getPublicKey();
+ }
+ return extraKeys.hasNext();
+ }
+ public void remove()
+ {
+ throw new UnsupportedOperationException("Key ring iterators are \
immutable."); + }
+ };
+ }
+
+ public PGPPublicKey getPublicKey(long keyID)
+ {
+ PGPSecretKey secKey = getSecretKey(keyID);
+ if (secKey != null)
+ {
+ return secKey.getPublicKey();
+ }
+
+ for (Iterator it = getExtraPublicKeys(); it.hasNext();)
+ {
+ PGPPublicKey pk = (PGPPublicKey)it.next();
+ if (pk.getKeyID() == keyID)
+ {
+ return pk;
+ }
+ }
+
+ return null;
+ }
+
+ /**
* Replace the public key set on the secret ring with the corresponding key off \
the public ring.
*
* @param secretRing secret ring to be changed.
diff --git a/google3/third_party/java/bouncycastle_bcpg/v1_46/bcpg-jdk15-146/src/org/bouncycastle/openpgp/test/PGPKeyRingTest.java \
b/google3/third_party/java/bouncycastle_bcpg/v1_46/bcpg-jdk15-146/src/org/bouncycastle/openpgp/test/PGPKeyRingTest.java
index 3163a4a..3b7da6a 100644
--- a/google3/third_party/java/bouncycastle_bcpg/v1_46/bcpg-jdk15-146/src/org/bouncycastle/openpgp/test/PGPKeyRingTest.java
+++ b/google3/third_party/java/bouncycastle_bcpg/v1_46/bcpg-jdk15-146/src/org/bouncycastle/openpgp/test/PGPKeyRingTest.java
@@ -16,6 +16,7 @@ import org.bouncycastle.jce.spec.ElGamalParameterSpec;
import org.bouncycastle.openpgp.PGPEncryptedData;
import org.bouncycastle.openpgp.PGPException;
import org.bouncycastle.openpgp.PGPKeyPair;
+import org.bouncycastle.openpgp.PGPKeyRing;
import org.bouncycastle.openpgp.PGPKeyRingGenerator;
import org.bouncycastle.openpgp.PGPPrivateKey;
import org.bouncycastle.openpgp.PGPPublicKey;
@@ -1087,7 +1088,7 @@ public class PGPKeyRingTest
while (rIt.hasNext())
{
- PGPPublicKeyRing pgpPub = \
(PGPPublicKeyRing)rIt.next(); + PGPKeyRing pgpPub = \
(PGPPublicKeyRing)rIt.next();
count++;
@@ -1193,7 +1194,7 @@ public class PGPKeyRingTest
while (rIt.hasNext())
{
- PGPSecretKeyRing pgpSec = \
(PGPSecretKeyRing)rIt.next(); + PGPKeyRing pgpSec = \
(PGPSecretKeyRing)rIt.next();
count++;
@@ -1203,7 +1204,7 @@ public class PGPKeyRingTest
pgpSec = new PGPSecretKeyRing(bytes);
- Iterator it = pgpSec.getSecretKeys();
+ Iterator it = ((PGPSecretKeyRing) pgpSec).getSecretKeys();
while (it.hasNext())
{
keyCount++;
@@ -1309,7 +1310,7 @@ public class PGPKeyRingTest
while (rIt.hasNext())
{
- PGPPublicKeyRing pgpPub = (PGPPublicKeyRing)rIt.next();
+ PGPKeyRing pgpPub = (PGPPublicKeyRing)rIt.next();
count++;
@@ -1353,7 +1354,7 @@ public class PGPKeyRingTest
while (rIt.hasNext())
{
- PGPSecretKeyRing pgpSec = \
(PGPSecretKeyRing)rIt.next(); + PGPKeyRing pgpSec = \
(PGPSecretKeyRing)rIt.next();
count++;
@@ -1363,7 +1364,7 @@ public class PGPKeyRingTest
pgpSec = new PGPSecretKeyRing(bytes);
- Iterator it = pgpSec.getSecretKeys();
+ Iterator it = ((PGPSecretKeyRing) pgpSec).getSecretKeys();
while (it.hasNext())
{
keyCount++;
@@ -1432,7 +1433,7 @@ public class PGPKeyRingTest
while (rIt.hasNext())
{
- PGPPublicKeyRing pgpPub = \
(PGPPublicKeyRing)rIt.next(); + PGPKeyRing pgpPub = \
(PGPPublicKeyRing)rIt.next();
count++;
@@ -1474,7 +1475,7 @@ public class PGPKeyRingTest
while (rIt.hasNext())
{
- PGPSecretKeyRing pgpSec = \
(PGPSecretKeyRing)rIt.next(); + PGPKeyRing pgpSec = \
(PGPSecretKeyRing)rIt.next();
count++;
@@ -1484,7 +1485,7 @@ public class PGPKeyRingTest
pgpSec = new PGPSecretKeyRing(bytes);
- Iterator it = pgpSec.getSecretKeys();
+ Iterator it = ((PGPSecretKeyRing) pgpSec).getSecretKeys();
while (it.hasNext())
{
keyCount++;
@@ -1567,7 +1568,7 @@ public class PGPKeyRingTest
while (rIt.hasNext())
{
- PGPPublicKeyRing pgpPub = \
(PGPPublicKeyRing)rIt.next(); + PGPKeyRing pgpPub = \
(PGPPublicKeyRing)rIt.next();
count++;
@@ -1666,7 +1667,7 @@ public class PGPKeyRingTest
while (rIt.hasNext())
{
- PGPPublicKeyRing pgpPub = (PGPPublicKeyRing)rIt.next();
+ PGPKeyRing pgpPub = (PGPPublicKeyRing)rIt.next();
Iterator it = pgpPub.getPublicKeys();
while (it.hasNext())
{
@@ -1696,7 +1697,7 @@ public class PGPKeyRingTest
public void test7()
throws Exception
{
- PGPPublicKeyRing pgpPub = new PGPPublicKeyRing(pub7);
+ PGPKeyRing pgpPub = new PGPPublicKeyRing(pub7);
Iterator it = pgpPub.getPublicKeys();
PGPPublicKey masterKey = null;
@@ -1749,7 +1750,7 @@ public class PGPKeyRingTest
while (rIt.hasNext())
{
- PGPPublicKeyRing pgpPub = (PGPPublicKeyRing)rIt.next();
+ PGPKeyRing pgpPub = (PGPPublicKeyRing)rIt.next();
count++;
@@ -1789,7 +1790,7 @@ public class PGPKeyRingTest
while (rIt.hasNext())
{
- PGPSecretKeyRing pgpSec = (PGPSecretKeyRing)rIt.next();
+ PGPKeyRing pgpSec = (PGPSecretKeyRing)rIt.next();
count++;
@@ -1799,7 +1800,7 @@ public class PGPKeyRingTest
pgpSec = new PGPSecretKeyRing(bytes);
- Iterator it = pgpSec.getSecretKeys();
+ Iterator it = ((PGPSecretKeyRing) pgpSec).getSecretKeys();
while (it.hasNext())
{
keyCount++;
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic