Skip to content

Commit bd67ccd

Browse files
K0zkaK0zka
committed
few cleanups in CertServiceTest and CertService
Tests: - all tests are @test rather than having one test to call them, so they can be run one by one - tests that expect exception from a method fail if there is none - no longer extends TestCase so that the original method names could be kept as test Implementation: - include root cause in exceptions when possible - helps at troubleshuting - close readers Signed-off-by: Laszlo Hornyak <laszlo.hornyak@gmail.com>
1 parent 5420cbe commit bd67ccd

2 files changed

Lines changed: 80 additions & 98 deletions

File tree

‎server/src/org/apache/cloudstack/network/lb/CertServiceImpl.java‎

Lines changed: 27 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -24,16 +24,20 @@
2424
import com.cloud.user.dao.AccountDao;
2525
import com.cloud.utils.db.EntityManager;
2626
import com.cloud.utils.exception.CloudRuntimeException;
27+
2728
import org.apache.cloudstack.api.command.user.loadbalancer.DeleteSslCertCmd;
2829
import org.apache.cloudstack.api.command.user.loadbalancer.ListSslCertsCmd;
2930
import org.apache.cloudstack.api.command.user.loadbalancer.UploadSslCertCmd;
3031
import org.apache.cloudstack.api.response.SslCertResponse;
32+
3133
import com.cloud.exception.InvalidParameterValueException;
3234
import com.cloud.user.Account;
3335
import com.cloud.user.AccountManager;
3436
import com.cloud.utils.db.DB;
37+
3538
import org.apache.cloudstack.acl.SecurityChecker;
3639
import org.apache.cloudstack.context.CallContext;
40+
import org.apache.commons.io.IOUtils;
3741
import org.apache.log4j.Logger;
3842
import org.bouncycastle.jce.provider.BouncyCastleProvider;
3943
import org.bouncycastle.openssl.PEMReader;
@@ -45,6 +49,7 @@
4549
import javax.crypto.NoSuchPaddingException;
4650
import javax.ejb.Local;
4751
import javax.inject.Inject;
52+
4853
import java.io.IOException;
4954
import java.io.StringReader;
5055
import java.io.UnsupportedEncodingException;
@@ -228,7 +233,7 @@ private void validate(String _cert, String _key, String _password, String _chain
228233
}
229234

230235
} catch (IOException e) {
231-
throw new IllegalArgumentException("Parsing certificate/key failed: " + e.getMessage());
236+
throw new IllegalArgumentException("Parsing certificate/key failed: " + e.getMessage(), e);
232237
}
233238

234239
validateCert(cert, _chain != null? true: false);
@@ -273,15 +278,15 @@ private void validateCert(Certificate cert, boolean chain_present) {
273278
try {
274279
((X509Certificate)cert).checkValidity();
275280
} catch (Exception e) {
276-
throw new IllegalArgumentException("Certificate expired or not valid");
281+
throw new IllegalArgumentException("Certificate expired or not valid", e);
277282
}
278283

279284
if( !chain_present ) {
280285
PublicKey pubKey = cert.getPublicKey();
281286
try {
282287
cert.verify(pubKey);
283288
} catch (Exception e) {
284-
throw new IllegalArgumentException("No chain given and certificate not self signed");
289+
throw new IllegalArgumentException("No chain given and certificate not self signed", e);
285290
}
286291
}
287292
}
@@ -309,15 +314,15 @@ private void validateKeys(PublicKey pubKey, PrivateKey privKey) {
309314
throw new IllegalArgumentException("Bad public-private key");
310315

311316
} catch (BadPaddingException e) {
312-
throw new IllegalArgumentException("Bad public-private key");
317+
throw new IllegalArgumentException("Bad public-private key", e);
313318
} catch (IllegalBlockSizeException e) {
314-
throw new IllegalArgumentException("Bad public-private key");
319+
throw new IllegalArgumentException("Bad public-private key", e);
315320
} catch (NoSuchPaddingException e) {
316-
throw new IllegalArgumentException("Bad public-private key");
321+
throw new IllegalArgumentException("Bad public-private key", e);
317322
} catch (InvalidKeyException e) {
318-
throw new IllegalArgumentException("Invalid public-private key");
323+
throw new IllegalArgumentException("Invalid public-private key", e);
319324
} catch (NoSuchAlgorithmException e) {
320-
throw new IllegalArgumentException("Invalid algorithm for public-private key");
325+
throw new IllegalArgumentException("Invalid algorithm for public-private key", e);
321326
}
322327
}
323328

@@ -363,11 +368,11 @@ private void validateChain(List<Certificate> chain, Certificate cert) {
363368
CertPathBuilder builder = CertPathBuilder.getInstance("PKIX");
364369
builder.build(params);
365370
} catch (InvalidAlgorithmParameterException e) {
366-
throw new IllegalArgumentException("Invalid certificate chain",null);
371+
throw new IllegalArgumentException("Invalid certificate chain", e);
367372
} catch (CertPathBuilderException e) {
368-
throw new IllegalArgumentException("Invalid certificate chain",null);
373+
throw new IllegalArgumentException("Invalid certificate chain", e);
369374
} catch (NoSuchAlgorithmException e) {
370-
throw new IllegalArgumentException("Invalid certificate chain",null);
375+
throw new IllegalArgumentException("Invalid certificate chain", e);
371376
}
372377

373378
}
@@ -380,7 +385,12 @@ public PrivateKey parsePrivateKey(String key, String password) throws IOExceptio
380385
pGet = new KeyPassword(password.toCharArray());
381386

382387
PEMReader privateKey = new PEMReader(new StringReader(key), pGet);
383-
Object obj = privateKey.readObject();
388+
Object obj = null;
389+
try {
390+
obj = privateKey.readObject();
391+
} finally {
392+
IOUtils.closeQuietly(privateKey);
393+
}
384394

385395
try {
386396

@@ -389,9 +399,8 @@ public PrivateKey parsePrivateKey(String key, String password) throws IOExceptio
389399

390400
return (PrivateKey) obj;
391401

392-
}catch (Exception e){
393-
e.printStackTrace();
394-
throw new IOException("Invalid Key format or invalid password.");
402+
} catch (Exception e){
403+
throw new IOException("Invalid Key format or invalid password.", e);
395404
}
396405
}
397406

@@ -402,6 +411,8 @@ public Certificate parseCertificate(String cert) {
402411
return (Certificate) certPem.readObject();
403412
} catch (Exception e) {
404413
throw new InvalidParameterValueException("Invalid Certificate format. Expected X509 certificate");
414+
} finally {
415+
IOUtils.closeQuietly(certPem);
405416
}
406417
}
407418

@@ -419,7 +430,7 @@ public List<Certificate> parseChain(String chain) throws IOException {
419430
}
420431
}
421432
if ( certs.size() == 0 )
422-
throw new IllegalArgumentException("Unable to decode certificate chain",null);
433+
throw new IllegalArgumentException("Unable to decode certificate chain");
423434

424435
return certs;
425436
}

0 commit comments

Comments
 (0)