[fix] make sure Context.ciphers are not mutated

when setting an invalid ciphers value (closing #219)

Author: kares

src/main/java/org/jruby/ext/openssl/SSLContext.java

@@ -518,26 +518,26 @@ public IRubyObject setup(final ThreadContext context) {
 
     @JRubyMethod
     public RubyArray ciphers(final ThreadContext context) { // SSL_CTX_get_ciphers
-        return matchedCiphersWithCache(context);
+        return matchedCiphersWithCache(context, this.ciphers);
     }
 
-    private RubyArray matchedCiphersWithCache(final ThreadContext context) {
+    private RubyArray matchedCiphersWithCache(final ThreadContext context, final String ciphers) {
         final CipherListCache cache = cipherListCache;
         if ( protocol.equals(cache.protocol) && ciphers.equals(cache.ciphers) ) {
             return newSharedArray(cache.cipherList);
         }
 
-        final RubyArray match = matchedCiphers(context);
+        final RubyArray match = matchedCiphers(context, ciphers);
         cipherListCache = new CipherListCache(protocol, ciphers, match);
         return newSharedArray(match);
     }
 
-    private RubyArray matchedCiphers(final ThreadContext context) {
+    private RubyArray matchedCiphers(final ThreadContext context, final String ciphers) {
         final Ruby runtime = context.runtime;
         try {
             final String[] supported = getSupportedCipherSuites(context, protocol);
             final Collection<CipherStrings.Def> cipherDefs =
-                    CipherStrings.matchingCiphers(this.ciphers, supported, false);
+                    CipherStrings.matchingCiphers(ciphers, supported, false);
 
             final IRubyObject[] cipherList = new IRubyObject[ cipherDefs.size() ];
 
@@ -562,8 +562,9 @@ private static RubyArray newSharedArray(RubyArray array) {
 
     @JRubyMethod(name = "ciphers=")
     public IRubyObject set_ciphers(final ThreadContext context, final IRubyObject ciphers) {
+        String cipherString;
         if ( ciphers.isNil() ) {
-            this.ciphers = CipherStrings.SSL_DEFAULT_CIPHER_LIST;
+            cipherString = CipherStrings.SSL_DEFAULT_CIPHER_LIST;
         }
         else if ( ciphers instanceof RubyArray ) {
             final RubyArray ciphs = (RubyArray) ciphers;
@@ -581,20 +582,23 @@ else if ( ciphers instanceof RubyArray ) {
                 cipherStr.append(sep).append( elem.toString() );
                 sep = ":";
             }
-            this.ciphers = cipherStr.toString();
+            cipherString = cipherStr.toString();
         }
         else {
-            this.ciphers = ciphers.asString().toString();
+            cipherString = ciphers.asString().toString();
         }
 
-        if (this.ciphers.equals(CipherStrings.SSL_DEFAULT_CIPHER_LIST)) {
-            this.ciphers = CipherStrings.SSL_DEFAULT_CIPHER_LIST; // due caching
+        if (cipherString.equals(CipherStrings.SSL_DEFAULT_CIPHER_LIST)) {
+            cipherString = CipherStrings.SSL_DEFAULT_CIPHER_LIST; // due caching
         }
 
-        if ( matchedCiphersWithCache(context).isEmpty() ) {
+        System.out.println("cipherString: " + cipherString + " " + matchedCiphersWithCache(context, cipherString).isEmpty());
+
+        if ( matchedCiphersWithCache(context, cipherString).isEmpty() ) {
             throw newSSLError(context.runtime, "no cipher match");
         }
 
+        this.ciphers = cipherString;
         return ciphers;
     }
 

src/test/ruby/ssl/test_context.rb

@@ -232,4 +232,16 @@ def test_set_ciphers_empty_array
     assert_include ex.message, "no cipher match"
   end
 
+  def test_invalid_ciphers_does_not_mutate_context
+    context = OpenSSL::SSL::SSLContext.new
+    ciphers = context.ciphers
+    assert !ciphers.empty?
+    begin
+      context.ciphers = ['AES256-SHA123']
+      fail 'raise expected'
+    rescue OpenSSL::SSL::SSLError
+    end
+    assert_equal context.ciphers, ciphers
+  end
+
 end