Skip to content

Commit 3336fb8

Browse files
kareskares
committed
post review #124 - mostly to follow JRuby ext best-practices + dry-out
1 parent 96955ce commit 3336fb8

File tree

6 files changed

+63
-77
lines changed

6 files changed

+63
-77
lines changed

‎src/main/java/org/jruby/ext/openssl/OCSP.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,7 @@ public static String getResponseStringForValue(IRubyObject fixnum) {
204204
return responseMap.get((int)rubyFixnum.getLongValue());
205205
}
206206

207-
public static RaiseException newOCSPError(Ruby runtime, Exception ex) {
207+
static RaiseException newOCSPError(Ruby runtime, Exception ex) {
208208
return Utils.newError(runtime, _OCSP(runtime).getClass("OCSPError"), ex);
209209
}
210210

‎src/main/java/org/jruby/ext/openssl/OCSPBasicResponse.java‎

Lines changed: 27 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@
8383

8484
import static org.jruby.ext.openssl.Digest._Digest;
8585
import static org.jruby.ext.openssl.OCSP._OCSP;
86+
import static org.jruby.ext.openssl.OCSP.newOCSPError;
8687
import static org.jruby.ext.openssl.X509._X509;
8788

8889
import java.io.IOException;
@@ -105,6 +106,7 @@
105106
*/
106107
public class OCSPBasicResponse extends RubyObject {
107108
private static final long serialVersionUID = 8755480816625884227L;
109+
108110
private static final String OCSP_NOCERTS = "NOCERTS";
109111
private static final String OCSP_NOCHAIN = "NOCHAIN";
110112
private static final String OCSP_NOCHECKS = "NOCHECKS";
@@ -121,9 +123,9 @@ public IRubyObject allocate(Ruby runtime, RubyClass klass) {
121123
}
122124
};
123125

124-
public static void createBasicResponse(final Ruby runtime, final RubyModule _OCSP) {
125-
RubyClass _BasicResponse = _OCSP.defineClassUnder("BasicResponse", runtime.getObject(), BASICRESPONSE_ALLOCATOR);
126-
_BasicResponse.defineAnnotatedMethods(OCSPBasicResponse.class);
126+
public static void createBasicResponse(final Ruby runtime, final RubyModule OCSP) {
127+
RubyClass BasicResponse = OCSP.defineClassUnder("BasicResponse", runtime.getObject(), BASICRESPONSE_ALLOCATOR);
128+
BasicResponse.defineAnnotatedMethods(OCSPBasicResponse.class);
127129
}
128130

129131
private byte[] nonce;
@@ -371,7 +373,7 @@ public IRubyObject sign(final ThreadContext context, IRubyObject[] args) {
371373

372374
@JRubyMethod(name = "verify", rest = true)
373375
public IRubyObject verify(final ThreadContext context, IRubyObject[] args) {
374-
Ruby runtime = getRuntime();
376+
Ruby runtime = context.runtime;
375377
int flags = 0;
376378
IRubyObject certificates = args[0];
377379
IRubyObject store = args[1];
@@ -385,7 +387,7 @@ public IRubyObject verify(final ThreadContext context, IRubyObject[] args) {
385387
jcacvpb.setProvider("BC");
386388
BasicOCSPResp basicOCSPResp = getBasicOCSPResp();
387389

388-
java.security.cert.Certificate signer = findSignerCert(asn1BCBasicOCSPResp, convertRubyCerts(certificates), flags);
390+
java.security.cert.Certificate signer = findSignerCert(context, asn1BCBasicOCSPResp, convertRubyCerts(certificates), flags);
389391
if ( signer == null ) return RubyBoolean.newBoolean(runtime, false);
390392
if ( (flags & RubyFixnum.fix2int((RubyFixnum)_OCSP(runtime).getConstant(OCSP_NOINTERN))) == 0 &&
391393
(flags & RubyFixnum.fix2int((RubyFixnum)_OCSP(runtime).getConstant(OCSP_TRUSTOTHER))) != 0 ) {
@@ -426,10 +428,9 @@ else if (basicOCSPResp.getCerts() != null && (certificates != null && !((RubyArr
426428
RubyArray rUntrustedCerts = RubyArray.newEmptyArray(runtime);
427429
if (untrustedCerts != null) {
428430
X509Cert[] rubyCerts = new X509Cert[untrustedCerts.size()];
429-
untrustedCerts.toArray(rubyCerts);
430-
rUntrustedCerts = RubyArray.newArray(runtime, rubyCerts);
431+
rUntrustedCerts = RubyArray.newArray(runtime, untrustedCerts.toArray(rubyCerts));
431432
}
432-
X509StoreContext ctx = null;
433+
X509StoreContext ctx;
433434
try {
434435
ctx = X509StoreContext.newStoreContext(context, (X509Store)store, X509Cert.wrap(runtime, signer), rUntrustedCerts);
435436
}
@@ -438,7 +439,7 @@ else if (basicOCSPResp.getCerts() != null && (certificates != null && !((RubyArr
438439
}
439440

440441
ctx.set_purpose(context, _X509(runtime).getConstant("PURPOSE_OCSP_HELPER"));
441-
ret = ((RubyBoolean)ctx.verify(context)).isTrue();
442+
ret = ctx.verify(context).isTrue();
442443
IRubyObject chain = ctx.chain(context);
443444

444445
if ((flags & RubyFixnum.fix2int((RubyFixnum)_OCSP(runtime).getConstant(OCSP_NOCHECKS))) > 0) {
@@ -473,20 +474,21 @@ else if (basicOCSPResp.getCerts() != null && (certificates != null && !((RubyArr
473474
}
474475

475476
@JRubyMethod(name = "status")
476-
public IRubyObject status() {
477-
Ruby runtime = getRuntime();
478-
RubyArray ret = RubyArray.newEmptyArray(runtime);
477+
public IRubyObject status(ThreadContext context) {
478+
final Ruby runtime = context.runtime;
479+
RubyArray ret = RubyArray.newArray(runtime, singleResponses.size());
479480

480481
for (OCSPSingleResponse resp : singleResponses) {
481-
RubyArray respAry = RubyArray.newEmptyArray(runtime);
482+
RubyArray respAry = RubyArray.newArray(runtime, 7);
482483

483-
respAry.add(resp.certid());
484-
respAry.add(resp.cert_status());
485-
respAry.add(resp.revocation_reason());
486-
respAry.add(resp.revocation_time());
487-
respAry.add(resp.this_update());
488-
respAry.add(resp.next_update());
489-
respAry.add(resp.extensions());
484+
respAry.append(resp.certid(context));
485+
respAry.append(resp.cert_status());
486+
respAry.append(resp.revocation_reason());
487+
respAry.append(resp.revocation_time());
488+
respAry.append(resp.this_update());
489+
respAry.append(resp.next_update());
490+
respAry.append(resp.extensions());
491+
490492
ret.add(respAry);
491493
}
492494

@@ -496,7 +498,7 @@ public IRubyObject status() {
496498
@JRubyMethod(name = "to_der")
497499
public IRubyObject to_der() {
498500
Ruby runtime = getRuntime();
499-
IRubyObject ret = null;
501+
IRubyObject ret;
500502
try {
501503
ret = RubyString.newString(runtime, asn1BCBasicOCSPResp.getEncoded());
502504
}
@@ -630,16 +632,12 @@ private List<java.security.cert.Certificate> convertRubyCerts(IRubyObject certif
630632

631633
return ret;
632634
}
633-
634-
private static RaiseException newOCSPError(Ruby runtime, Exception e) {
635-
return Utils.newError(runtime, _OCSP(runtime).getClass("OCSPError"), e);
636-
}
637635

638-
private java.security.cert.Certificate findSignerCert(BasicOCSPResponse basicResp, List<java.security.cert.Certificate> certificates, int flags) {
639-
Ruby runtime = getRuntime();
640-
ThreadContext context = runtime.getCurrentContext();
636+
private java.security.cert.Certificate findSignerCert(final ThreadContext context,
637+
BasicOCSPResponse basicResp, List<java.security.cert.Certificate> certificates, int flags) {
638+
final Ruby runtime = context.runtime;
641639
ResponderID respID = basicResp.getTbsResponseData().getResponderID();
642-
java.security.cert.Certificate ret = null;
640+
java.security.cert.Certificate ret;
643641
ret = findSignerByRespId(context, certificates, respID);
644642

645643
if (ret == null && (flags & RubyFixnum.fix2int((RubyFixnum)_OCSP(runtime).getConstant(OCSP_NOINTERN))) == 0) {

‎src/main/java/org/jruby/ext/openssl/OCSPCertificateId.java‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@
6060

6161
import static org.jruby.ext.openssl.OCSP._OCSP;
6262
import static org.jruby.ext.openssl.Digest._Digest;
63+
import static org.jruby.ext.openssl.OCSP.newOCSPError;
6364

6465
/**
6566
* An OpenSSL::OCSP::CertificateId identifies a certificate to the
@@ -314,8 +315,4 @@ public CertificateID getBCCertificateID() {
314315
return new CertificateID(bcCertId);
315316
}
316317

317-
private static RaiseException newOCSPError(Ruby runtime, Exception e) {
318-
return Utils.newError(runtime, _OCSP(runtime).getClass("OCSPError"), e);
319-
}
320-
321318
}

‎src/main/java/org/jruby/ext/openssl/OCSPRequest.java‎

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@
3434

3535
import static org.jruby.ext.openssl.Digest._Digest;
3636
import static org.jruby.ext.openssl.OCSP._OCSP;
37+
import static org.jruby.ext.openssl.OCSP.newOCSPError;
38+
import static org.jruby.ext.openssl.OpenSSL.debugStackTrace;
3739
import static org.jruby.ext.openssl.X509._X509;
3840

3941
import java.io.IOException;
@@ -205,15 +207,15 @@ public IRubyObject certid() {
205207
}
206208

207209
@JRubyMethod(name = "check_nonce")
208-
public IRubyObject check_nonce(IRubyObject response) {
209-
Ruby runtime = getRuntime();
210+
public IRubyObject check_nonce(ThreadContext context, IRubyObject response) {
211+
final Ruby runtime = context.runtime;
210212
if (response instanceof OCSPBasicResponse) {
211213
OCSPBasicResponse rubyBasicRes = (OCSPBasicResponse) response;
212214
return checkNonceImpl(runtime, this.nonce, rubyBasicRes.getNonce());
213215
}
214216
else if (response instanceof OCSPResponse) {
215217
OCSPResponse rubyResp = (OCSPResponse) response;
216-
return checkNonceImpl(runtime, this.nonce, ((OCSPBasicResponse)rubyResp.basic()).getNonce());
218+
return checkNonceImpl(runtime, this.nonce, ((OCSPBasicResponse)rubyResp.basic(context)).getNonce());
217219
}
218220
else {
219221
return checkNonceImpl(runtime, this.nonce, null);
@@ -222,7 +224,7 @@ else if (response instanceof OCSPResponse) {
222224

223225
@JRubyMethod(name = "sign", rest = true)
224226
public IRubyObject sign(final ThreadContext context, IRubyObject[] args) {
225-
Ruby runtime = context.getRuntime();
227+
final Ruby runtime = context.runtime;
226228

227229
int flag = 0;
228230
IRubyObject additionalCerts = context.nil;
@@ -380,8 +382,8 @@ public IRubyObject verify(IRubyObject[] args) {
380382
if (!ret) return RubyBoolean.newBoolean(runtime, false);
381383
}
382384
}
383-
catch ( Exception e ) {
384-
e.printStackTrace();
385+
catch (Exception e) {
386+
debugStackTrace(e);
385387
throw newOCSPError(runtime, e);
386388
}
387389

@@ -473,8 +475,5 @@ public OCSPReq getBCOCSPReq() {
473475
if (asn1bcReq == null) return null;
474476
return new OCSPReq(asn1bcReq);
475477
}
476-
private static RaiseException newOCSPError(Ruby runtime, Exception e) {
477-
return Utils.newError(runtime, _OCSP(runtime).getClass("OCSPError"), e);
478-
}
479478

480479
}

‎src/main/java/org/jruby/ext/openssl/OCSPResponse.java‎

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
package org.jruby.ext.openssl;
3434

3535
import static org.jruby.ext.openssl.OCSP._OCSP;
36+
import static org.jruby.ext.openssl.OCSP.newOCSPError;
3637

3738
import java.io.IOException;
3839

@@ -77,9 +78,9 @@ public OCSPResponse(Ruby runtime) {
7778
this(runtime, (RubyClass) _OCSP(runtime).getConstantAt("Response"));
7879
}
7980

80-
public static void createResponse(final Ruby runtime, final RubyModule _OCSP) {
81-
RubyClass _request = _OCSP.defineClassUnder("Response", runtime.getObject(), RESPONSE_ALLOCATOR);
82-
_request.defineAnnotatedMethods(OCSPResponse.class);
81+
public static void createResponse(final Ruby runtime, final RubyModule OCSP) {
82+
RubyClass Response = OCSP.defineClassUnder("Response", runtime.getObject(), RESPONSE_ALLOCATOR);
83+
Response.defineAnnotatedMethods(OCSPResponse.class);
8384
}
8485

8586
private org.bouncycastle.asn1.ocsp.OCSPResponse bcResp;
@@ -151,11 +152,10 @@ public IRubyObject initialize_copy(IRubyObject obj) {
151152
}
152153

153154
@JRubyMethod(name = "basic")
154-
public IRubyObject basic() {
155-
Ruby runtime = getRuntime();
156-
ThreadContext context = runtime.getCurrentContext();
155+
public IRubyObject basic(ThreadContext context) {
156+
Ruby runtime = context.runtime;
157157
if (bcResp == null || bcResp.getResponseBytes() == null || bcResp.getResponseBytes().getResponse() == null) {
158-
return getRuntime().getCurrentContext().nil;
158+
return context.nil;
159159
}
160160
else {
161161
OCSPBasicResponse ret = new OCSPBasicResponse(runtime);
@@ -188,9 +188,5 @@ public IRubyObject to_der() {
188188
public org.bouncycastle.asn1.ocsp.OCSPResponse getBCResp() {
189189
return bcResp;
190190
}
191-
192-
private static RaiseException newOCSPError(Ruby runtime, Exception e) {
193-
return Utils.newError(runtime, _OCSP(runtime).getClass("OCSPError"), e);
194-
}
195191

196192
}

‎src/main/java/org/jruby/ext/openssl/OCSPSingleResponse.java‎

Lines changed: 19 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
package org.jruby.ext.openssl;
3434

3535
import static org.jruby.ext.openssl.OCSP._OCSP;
36+
import static org.jruby.ext.openssl.OCSP.newOCSPError;
3637

3738
import java.io.IOException;
3839
import java.text.ParseException;
@@ -117,9 +118,8 @@ public IRubyObject cert_status() {
117118
}
118119

119120
@JRubyMethod(name = "certid")
120-
public IRubyObject certid() {
121-
Ruby runtime = getRuntime();
122-
ThreadContext context = runtime.getCurrentContext();
121+
public IRubyObject certid(ThreadContext context) {
122+
Ruby runtime = context.runtime;
123123
CertID bcCertId = bcSingleResponse.getCertID();
124124
OCSPCertificateId rubyCertId = new OCSPCertificateId(runtime);
125125
try {
@@ -182,22 +182,21 @@ public IRubyObject extensions() {
182182
Ruby runtime = getRuntime();
183183
Extensions exts = bcSingleResponse.getSingleExtensions();
184184
if (exts == null) return RubyArray.newEmptyArray(runtime);
185-
List<X509Extension> retExts = new ArrayList<X509Extension>();
186-
List<ASN1ObjectIdentifier> extOids = Arrays.asList(exts.getExtensionOIDs());
187-
for (ASN1ObjectIdentifier extOid : extOids) {
188-
Extension ext = exts.getExtension(extOid);
185+
ASN1ObjectIdentifier[] extOIDs = exts.getExtensionOIDs();
186+
RubyArray retExts = runtime.newArray(extOIDs.length);
187+
for (ASN1ObjectIdentifier extOID : extOIDs) {
188+
Extension ext = exts.getExtension(extOID);
189189
ASN1Encodable extAsn1 = ext.getParsedValue();
190-
X509Extension retExt = X509Extension.newExtension(runtime, extOid, extAsn1, ext.isCritical());
191-
retExts.add(retExt);
190+
X509Extension retExt = X509Extension.newExtension(runtime, extOID, extAsn1, ext.isCritical());
191+
retExts.append(retExt);
192192
}
193-
194-
return RubyArray.newArray(runtime, retExts);
193+
return retExts;
195194
}
196195

197196
@JRubyMethod(name = "next_update")
198197
public IRubyObject next_update() {
199198
Ruby runtime = getRuntime();
200-
if (bcSingleResponse.getNextUpdate() == null) return runtime.getCurrentContext().nil;
199+
if (bcSingleResponse.getNextUpdate() == null) return runtime.getNil();
201200
Date nextUpdate;
202201
try {
203202
nextUpdate = bcSingleResponse.getNextUpdate().getDate();
@@ -207,7 +206,7 @@ public IRubyObject next_update() {
207206
}
208207

209208
if (nextUpdate == null) {
210-
return runtime.getCurrentContext().nil;
209+
return runtime.getNil();
211210
}
212211

213212
return RubyTime.newTime(runtime, nextUpdate.getTime());
@@ -216,7 +215,7 @@ public IRubyObject next_update() {
216215
@JRubyMethod(name = "this_update")
217216
public IRubyObject this_update() {
218217
Ruby runtime = getRuntime();
219-
if (bcSingleResponse.getThisUpdate() == null) return runtime.getCurrentContext().nil;
218+
if (bcSingleResponse.getThisUpdate() == null) return runtime.getNil();
220219
Date thisUpdate;
221220
try {
222221
thisUpdate = bcSingleResponse.getThisUpdate().getDate();
@@ -232,18 +231,18 @@ public IRubyObject this_update() {
232231
public IRubyObject revocation_reason() {
233232
Ruby runtime = getRuntime();
234233
RubyFixnum revoked = (RubyFixnum) _OCSP(runtime).getConstant("V_CERTSTATUS_REVOKED");
235-
if (bcSingleResponse.getCertStatus().getTagNo() == (int)revoked.getLongValue()) {
234+
if (bcSingleResponse.getCertStatus().getTagNo() == (int) revoked.getLongValue()) {
236235
try {
237236
RevokedInfo revokedInfo = RevokedInfo.getInstance(
238-
DERTaggedObject.fromByteArray(bcSingleResponse.getCertStatus().getStatus().toASN1Primitive().getEncoded())
239-
);
237+
DERTaggedObject.fromByteArray(bcSingleResponse.getCertStatus().getStatus().toASN1Primitive().getEncoded())
238+
);
240239
return RubyFixnum.newFixnum(runtime, revokedInfo.getRevocationReason().getValue().intValue());
241240
}
242241
catch (IOException e) {
243242
throw newOCSPError(runtime, e);
244243
}
245244
}
246-
return runtime.getCurrentContext().nil;
245+
return runtime.getNil();
247246
}
248247

249248
@JRubyMethod(name = "revocation_time")
@@ -261,7 +260,7 @@ public IRubyObject revocation_time() {
261260
throw newOCSPError(runtime, e);
262261
}
263262
}
264-
return runtime.getCurrentContext().nil;
263+
return runtime.getNil();
265264
}
266265

267266
@JRubyMethod(name = "to_der")
@@ -312,8 +311,5 @@ private boolean checkValidityImpl(Date thisUpdate, Date nextUpdate, int nsec, in
312311

313312
return ret;
314313
}
315-
316-
private static RaiseException newOCSPError(Ruby runtime, Exception e) {
317-
return Utils.newError(runtime, _OCSP(runtime).getClass("OCSPError"), e);
318-
}
314+
319315
}

0 commit comments

Comments
 (0)