Avoid unnecessary ByteBuffer.wrapping of GPG fingerprints
We can get equivalent behavior from JGit's NB utility functions
without the additional allocation. Consolidate the toString
implementation for fingerprints into a static method in Fingerprint.
Change-Id: I42d6398cdc40cbc3d145065c32b4172d9d8f067d
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/AccountIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/AccountIT.java
index e1b98af..91d3f11 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/AccountIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/AccountIT.java
@@ -17,7 +17,6 @@
import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.Truth.assert_;
-import static com.google.gerrit.server.git.gpg.PublicKeyStore.fingerprintToString;
import static com.google.gerrit.server.git.gpg.PublicKeyStore.keyToString;
import static java.nio.charset.StandardCharsets.UTF_8;
@@ -40,6 +39,7 @@
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.GpgKeys;
import com.google.gerrit.server.config.AllUsersName;
+import com.google.gerrit.server.git.gpg.Fingerprint;
import com.google.gerrit.server.git.gpg.PublicKeyStore;
import com.google.gerrit.server.git.gpg.TestKey;
import com.google.inject.Inject;
@@ -360,7 +360,7 @@
assertKeyEquals(key, gApi.accounts().self().gpgKey(
key.getKeyIdString()).get());
assertKeyEquals(key, gApi.accounts().self().gpgKey(
- fingerprintToString(key.getPublicKey().getFingerprint())).get());
+ Fingerprint.toString(key.getPublicKey().getFingerprint())).get());
assertKeyMapContains(key, keyMap);
}
@@ -395,7 +395,7 @@
String id = expected.getKeyIdString();
assertThat(actual.id).named(id).isEqualTo(id);
assertThat(actual.fingerprint).named(id).isEqualTo(
- fingerprintToString(expected.getPublicKey().getFingerprint()));
+ Fingerprint.toString(expected.getPublicKey().getFingerprint()));
@SuppressWarnings("unchecked")
List<String> userIds =
ImmutableList.copyOf(expected.getPublicKey().getUserIDs());
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/GpgKeys.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/GpgKeys.java
index 846db54..bbcdd06 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/GpgKeys.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/GpgKeys.java
@@ -35,6 +35,7 @@
import com.google.gerrit.reviewdb.client.AccountExternalId;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.account.AccountResource.GpgKey;
+import com.google.gerrit.server.git.gpg.Fingerprint;
import com.google.gerrit.server.git.gpg.PublicKeyStore;
import com.google.gerrit.server.util.BouncyCastleUtil;
import com.google.gwtorm.server.OrmException;
@@ -46,12 +47,12 @@
import org.bouncycastle.openpgp.PGPException;
import org.bouncycastle.openpgp.PGPPublicKey;
import org.bouncycastle.openpgp.PGPPublicKeyRing;
+import org.eclipse.jgit.util.NB;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
-import java.nio.ByteBuffer;
import java.util.Arrays;
import java.util.HashMap;
import java.util.Iterator;
@@ -161,7 +162,8 @@
}
}
if (!found) {
- log.warn("No public key stored for fingerprint {}", fp);
+ log.warn("No public key stored for fingerprint {}",
+ Fingerprint.toString(fp));
}
}
}
@@ -196,7 +198,7 @@
}
private static long keyId(byte[] fp) {
- return ByteBuffer.wrap(fp).getLong(fp.length - 8);
+ return NB.decodeInt64(fp, fp.length - 8);
}
static void checkEnabled() throws ResourceNotFoundException {
@@ -209,7 +211,7 @@
PGPPublicKey key = keyRing.getPublicKey();
GpgKeyInfo info = new GpgKeyInfo();
info.id = PublicKeyStore.keyIdToString(key.getKeyID());
- info.fingerprint = PublicKeyStore.fingerprintToString(key.getFingerprint());
+ info.fingerprint = Fingerprint.toString(key.getFingerprint());
@SuppressWarnings("unchecked")
Iterator<String> userIds = key.getUserIDs();
info.userIds = ImmutableList.copyOf(userIds);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/gpg/Fingerprint.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/gpg/Fingerprint.java
index 0649b93..924f918 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/gpg/Fingerprint.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/gpg/Fingerprint.java
@@ -18,18 +18,31 @@
import org.eclipse.jgit.util.NB;
-import java.nio.ByteBuffer;
import java.util.Arrays;
public class Fingerprint {
private final byte[] fp;
+ public static String toString(byte[] fp) {
+ checkLength(fp);
+ return String.format(
+ "%04X %04X %04X %04X %04X %04X %04X %04X %04X %04X",
+ NB.decodeUInt16(fp, 0), NB.decodeUInt16(fp, 2), NB.decodeUInt16(fp, 4),
+ NB.decodeUInt16(fp, 6), NB.decodeUInt16(fp, 8), NB.decodeUInt16(fp, 10),
+ NB.decodeUInt16(fp, 12), NB.decodeUInt16(fp, 14),
+ NB.decodeUInt16(fp, 16), NB.decodeUInt16(fp, 18));
+ }
+
+ private static byte[] checkLength(byte[] fp) {
+ checkArgument(fp.length == 20,
+ "fingerprint must be 20 bytes, got %s", fp.length);
+ return fp;
+ }
+
public Fingerprint(byte[] fp) {
// Don't bother with defensive copies; PGPPublicKey#getFingerprint() already
// does so.
- checkArgument(fp.length == 20,
- "fingerprint must be 20 bytes, got %s", fp.length);
- this.fp = fp;
+ this.fp = checkLength(fp);
}
public byte[] get() {
@@ -53,15 +66,10 @@
@Override
public String toString() {
- ByteBuffer buf = ByteBuffer.wrap(fp);
- return String.format(
- "(%04X %04X %04X %04X %04X %04X %04X %04X %04X %04X)",
- buf.getShort(), buf.getShort(), buf.getShort(), buf.getShort(),
- buf.getShort(), buf.getShort(), buf.getShort(), buf.getShort(),
- buf.getShort(), buf.getShort());
+ return toString(fp);
}
public long getId() {
- return ByteBuffer.wrap(fp).getLong(12);
+ return NB.decodeInt64(fp, 12);
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/gpg/PublicKeyStore.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/gpg/PublicKeyStore.java
index b216281..7151914 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/gpg/PublicKeyStore.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/gpg/PublicKeyStore.java
@@ -38,11 +38,11 @@
import org.eclipse.jgit.notes.NoteMap;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevWalk;
+import org.eclipse.jgit.util.NB;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.io.InputStream;
-import java.nio.ByteBuffer;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
@@ -335,7 +335,7 @@
"%s %s(%s)",
keyIdToString(key.getKeyID()),
it.hasNext() ? it.next() + " " : "",
- fingerprintToString(key.getFingerprint()));
+ Fingerprint.toString(key.getFingerprint()));
}
public static String keyIdToString(long keyId) {
@@ -343,21 +343,9 @@
return String.format("%08X", (int) keyId);
}
- public static String fingerprintToString(byte[] fingerprint) {
- if (fingerprint.length != 20) {
- throw new IllegalArgumentException();
- }
- ByteBuffer buf = ByteBuffer.wrap(fingerprint);
- return String.format(
- "%04X %04X %04X %04X %04X %04X %04X %04X %04X %04X",
- buf.getShort(), buf.getShort(), buf.getShort(), buf.getShort(),
- buf.getShort(), buf.getShort(), buf.getShort(), buf.getShort(),
- buf.getShort(), buf.getShort());
- }
-
static ObjectId keyObjectId(long keyId) {
- ByteBuffer buf = ByteBuffer.wrap(new byte[Constants.OBJECT_ID_LENGTH]);
- buf.putLong(keyId);
- return ObjectId.fromRaw(buf.array());
+ byte[] buf = new byte[Constants.OBJECT_ID_LENGTH];
+ NB.encodeInt64(buf, 0, keyId);
+ return ObjectId.fromRaw(buf);
}
}
