Cosmetic Cleanups for IpSecService
This is a follow-up CL to address comments
on aosp/466677
-Rename ManagedResourceArray.get()
-Comment cleanup
Bug: 38397094
Test: runtest frameworks-net
Change-Id: I6fbdd89c4a864fe1d8a19c68947f582d7b1f0f21
diff --git a/core/java/android/net/IpSecConfig.java b/core/java/android/net/IpSecConfig.java
index ceccc07..632b7fc 100644
--- a/core/java/android/net/IpSecConfig.java
+++ b/core/java/android/net/IpSecConfig.java
@@ -34,7 +34,7 @@
// Preventing this from being null simplifies Java->Native binder
private String mRemoteAddress = "";
- // The underlying network interface that represents the "gateway" Network
+ // The underlying Network that represents the "gateway" Network
// for outbound packets. It may also be used to select packets.
private Network mNetwork;
@@ -273,6 +273,7 @@
};
@VisibleForTesting
+ /** Equals method used for testing */
public static boolean equals(IpSecConfig lhs, IpSecConfig rhs) {
if (lhs == null || rhs == null) return (lhs == rhs);
return (lhs.mMode == rhs.mMode
diff --git a/core/java/android/net/IpSecTransform.java b/core/java/android/net/IpSecTransform.java
index 529cf1a..e15a2c6 100644
--- a/core/java/android/net/IpSecTransform.java
+++ b/core/java/android/net/IpSecTransform.java
@@ -409,8 +409,6 @@
public IpSecTransform buildTransportModeTransform(InetAddress remoteAddress)
throws IpSecManager.ResourceUnavailableException,
IpSecManager.SpiUnavailableException, IOException {
- //FIXME: argument validation here
- //throw new IllegalArgumentException("Natt Keepalive requires UDP Encapsulation");
mConfig.setMode(MODE_TRANSPORT);
mConfig.setRemoteAddress(remoteAddress.getHostAddress());
return new IpSecTransform(mContext, mConfig).activate();
diff --git a/services/core/java/com/android/server/IpSecService.java b/services/core/java/com/android/server/IpSecService.java
index 555874c..2e1f142 100644
--- a/services/core/java/com/android/server/IpSecService.java
+++ b/services/core/java/com/android/server/IpSecService.java
@@ -344,7 +344,7 @@
private class ManagedResourceArray<T extends ManagedResource> {
SparseArray<T> mArray = new SparseArray<>();
- T get(int key) {
+ T getAndCheckOwner(int key) {
T val = mArray.get(key);
// The value should never be null unless the resource doesn't exist
// (since we do not allow null resources to be added).
@@ -723,7 +723,7 @@
throws RemoteException {
// We want to non-destructively get so that we can check credentials before removing
// this from the records.
- T record = resArray.get(resourceId);
+ T record = resArray.getAndCheckOwner(resourceId);
if (record == null) {
throw new IllegalArgumentException(
@@ -863,7 +863,8 @@
break;
case IpSecTransform.ENCAP_ESPINUDP:
case IpSecTransform.ENCAP_ESPINUDP_NON_IKE:
- if (mUdpSocketRecords.get(config.getEncapSocketResourceId()) == null) {
+ if (mUdpSocketRecords.getAndCheckOwner(
+ config.getEncapSocketResourceId()) == null) {
throw new IllegalStateException(
"No Encapsulation socket for Resource Id: "
+ config.getEncapSocketResourceId());
@@ -885,7 +886,7 @@
throw new IllegalArgumentException("Encryption and Authentication are both null");
}
- if (mSpiRecords.get(config.getSpiResourceId(direction)) == null) {
+ if (mSpiRecords.getAndCheckOwner(config.getSpiResourceId(direction)) == null) {
throw new IllegalStateException("No SPI for specified Resource Id");
}
}
@@ -913,7 +914,7 @@
UdpSocketRecord socketRecord = null;
encapType = c.getEncapType();
if (encapType != IpSecTransform.ENCAP_NONE) {
- socketRecord = mUdpSocketRecords.get(c.getEncapSocketResourceId());
+ socketRecord = mUdpSocketRecords.getAndCheckOwner(c.getEncapSocketResourceId());
encapLocalPort = socketRecord.getPort();
encapRemotePort = c.getEncapRemotePort();
}
@@ -922,7 +923,7 @@
IpSecAlgorithm auth = c.getAuthentication(direction);
IpSecAlgorithm crypt = c.getEncryption(direction);
- spis[direction] = mSpiRecords.get(c.getSpiResourceId(direction));
+ spis[direction] = mSpiRecords.getAndCheckOwner(c.getSpiResourceId(direction));
int spi = spis[direction].getSpi();
try {
mSrvConfig
@@ -976,7 +977,7 @@
// Synchronize liberally here because we are using ManagedResources in this block
TransformRecord info;
// FIXME: this code should be factored out into a security check + getter
- info = mTransformRecords.get(resourceId);
+ info = mTransformRecords.getAndCheckOwner(resourceId);
if (info == null) {
throw new IllegalArgumentException("Transform " + resourceId + " is not active");