RubyString#toString() returns trimmed java.lang.String sequence when the given sequence is malformed

Since I wrote a question to Tom. Let me know when my guessing is wrong.

Tom, I'm very sorry for my wrong claim. The commit I pointed is not related to the bug. I thought we're invoking 'if (charset == null) {}' block in 1.8 mode but I was wrong, of course.

The bad(I should not use this word, maybe) commit is this;

commit 028579935f48d49f7e60c28d37422832ed4ef34f
Author: Charles Oliver Nutter <headius@headius.com>
Date:   Wed Jan 26 23:02:44 2011 -0600

    Fix JRUBY-5388: Requiring a filename with accented characters fails
    
    * When converting a require/load name to a Java String, be sure to decode using the encoding

It fixes RubyString#toString() to decode according to charset properly, and let RubyKernel#require use it for a file path.

Charles, how do you think about this issue? What JavaString RubyString#toString() should return? In 1.9 mode, it's clear that current decoding behavior is right I think. How about 1.8 mode?

Hope I'm talking to the right person this time...

I figured it was probably my change.

Ok...so this is a good find.

Originally, RubyString#toString decoded all strings using ISO-8859-1. 8859-1 is an 8-bit encoding, so this conversion did not lose any byte values; they would round-trip correctly. This is appropriate for binary data, but not appropriate for character data that may be some other encoding.

The tricky bit here is that 1.8 doesn't really have a clear notion of string encoding. There's the current KCODE, but even with that set binary data still needs to be treated as binary data.

Unfortunately toString wasn't even consistent with other Java integration use of RubyString. When passing a RubyString as a String, we did the current decoding logic (use string's encoding in 1.9, UTF-8 in 1.8). So depending on how you convert RubyString into String, it would follow different paths.

I think the root issue here is jruby-ossl expecting binary data decoded into a String to still be valid binary data. That seems like a stretch, since String is intended to hold character data decoded using some character set. Defaulting to UTF-8 may be incorrect on JRuby's part, but we have to pick something.

I'm not sure how to remedy this in JRuby.

I am inclined to leave the code as-is (perhaps with logic to use kcode encoding rather than UTF-8 unconditionally) and fix jruby-ossl instead, since I feel like there should be no expectation that binary data passed through java.lang.String is still valid binary data. If it's not character data, it should not be decoded into a character string.

Nahi: Where is the code that calls RubyString#toString?

Nahi: I made an attempt to fix this by decoding the Ruby string explicitly as 8859 within jruby-ossl, but I'm not sure I'm running the tests right. Here's the patch:

diff --git a/src/java/org/jruby/ext/openssl/PKeyRSA.java b/src/java/org/jruby/ext/openssl/PKeyRSA.java
index 60c2a94..36185c2 100644
--- a/src/java/org/jruby/ext/openssl/PKeyRSA.java
+++ b/src/java/org/jruby/ext/openssl/PKeyRSA.java
@@ -31,6 +31,8 @@ import java.io.IOException;
 import java.io.StringReader;
 import java.io.StringWriter;
 import java.math.BigInteger;
+import java.nio.ByteBuffer;
+import java.nio.charset.Charset;
 import java.security.GeneralSecurityException;
 import java.security.KeyFactory;
 import java.security.KeyPair;
@@ -163,6 +165,8 @@ public class PKeyRSA extends PKey {
         }
     }
 
+    private static final Charset ISO_8859_1 = Charset.forName("ISO-8859-1");
+
     @JRubyMethod(frame = true, rest = true)
     public IRubyObject initialize(IRubyObject[] args, Block block) {
         IRubyObject arg;
@@ -185,10 +189,10 @@ public class PKeyRSA extends PKey {
                 rsaGenerate(this, keysize, exp);
             } else {
                 if (pass != null && !pass.isNil()) {
-                    passwd = pass.toString().toCharArray();
+                    passwd = ISO_8859_1.decode(ByteBuffer.wrap(pass.convertToString().getByteList().bytes())).toString().toCharArray();
                 }
                 arg = OpenSSLImpl.to_der_if_possible(arg);
-                String input = arg.convertToString().toString();
+                String input = ISO_8859_1.decode(ByteBuffer.wrap(arg.convertToString().getByteList().bytes())).toString();
 
                 Object val = null;
                 KeyFactory fact = null;

And here's what I get when I attempt to run tests:

~/projects/jruby-ossl ➔ ant test
Buildfile: /Users/headius/projects/jruby-ossl/build.xml

init:

build:
    [javac] Compiling 1 source file to /Users/headius/projects/jruby-ossl/target/classes

jar:
      [jar] Building jar: /Users/headius/projects/jruby-ossl/lib/jopenssl.jar

test:
     [exec] OpenSSL ASN1/PKey/X509/Netscape/PKCS7 implementation unavailable
     [exec] gem install bouncy-castle-java for full support.
     [exec] OpenSSL::PKey::RSAError: Neither PUB key nor PRIV key:
     [exec]          initialize at org/jruby/ext/openssl/PKeyRSA.java:273
     [exec]   (class TestUtils) at ./test/openssl/utils.rb:5
     [exec]              (root) at ./test/openssl/utils.rb:4
     [exec]             require at org/jruby/RubyKernel.java:1037
     [exec]              (root) at ./test/openssl/utils.rb:3
     [exec]             require at org/jruby/RubyKernel.java:1037
     [exec]            __file__ at test/test_openssl.rb:3
     [exec]                each at org/jruby/RubyArray.java:1572
     [exec]              (root) at test/test_openssl.rb:2

BUILD FAILED
/Users/headius/projects/jruby-ossl/build.xml:54: exec returned: 1

Total time: 4 seconds

I think the technique I use here is a correct fix, but I'm not sure if I'm running the tests or fixing it in the right place. Can you suggest a course of action?

Charles: Thanks for the comment. I almost agree with the points you wrote in the comment at "22/Feb/11 1:03 AM". But in 1.8, the "String" of CRuby represents binary, so the java.lang.String value returned by RubyString#toString() could be treated as an ISO8859_1 sequence in 1.8 I thought. For JRUBY-5388, I thought require logic could invoke decodeString directly.

Aside from this, as I and you said JRuby-OSSL uses toString() in wrong way. It's expecting byte sequence even in 1.9 mode so it must be fixed. I'll push the next version of JRuby-OSSL before 1.6GA (and hopefully, not so soon from 1.6.0RC3.)

Fixed JRuby-OSSL at [master 8076b5f].

JRUBY-5519: Avoid String encoding dependency in DER loading

PKey::RSA.initialize and PKey::DSA.initialize invoked 
RubyString#toString() to get java.lang.String then used for PEM and DER
loading. It's proper for PEM loading since it's a String (though it
doesn't include non ASCII char) but we were utlized 
ByteList.plain(char[]) to get byte[] for DER loading. So here's a hidden 
unnecessary dependency to java.lang.String encoding.

Before JRuby 1.6.0 RC2 (1.5.x and 1.6.0RC1), RubyString#toString() 
returned java.lang.String with ISO8859_1 charset and the above logic 
*accidentaly* worked as expected. See JRUBY-5519 for the reason why 
1.6.0RC2 needs this fix.

The fix is simpley make it use RubyString#toString() for PEM loading and 
RubyString#getBytes() for DER loading. 

For build system including testing, we're using rake. I didn't aware of 'ant test' is broken. I'll fix it. Thanks!

If we want to switch completely to Rake, that's fine with me; I was confused which "test" target I should be running.

As far as your comment about ISO_8859_1:

Because a Java String represents characters, and because toString use used a lot for Java integration, we decided long ago to assume in 1.8 mode that RubyString.toString would use UTF-8 encoding. That seemed like a good decision at the time.

The truth, however, is that there's no perfect encoding to assume in 1.8 mode. If we assume UTF-8, then 8-bit ASCII strings are decoded improperly. If we assume ASCII-8BIT, UTF-8 strings are decoded improperly. If we use KCODE, then programs that mix encodings or also use binary strings are incorrect. In general, the most reasonable solution seemed to be to require that Strings entering Java be UTF-8.

In 1.9 mode, the situation is obviously much better, since we can know the string's intended encoding.

Bottom line then is that RubyString.toString should be expected to decode the String, either using UTF-8 or using the encoding attached to it in 1.9 mode. If you want the bytes, you should get the ByteList out.

I would not be opposed to adding a toByteString or toAsciiString or similar-named method, if it would make this clearer.

I agree that we should add a method for getting ISO_8859_1 string (this is our raw encoding in 1.8). In Ruby String is a binary store as well as a String, so it makes sense to have this method which ignores encoding.

I'm going to close this ticket;

• I fixed jruby-ossl for this issue. In any mean, jruby-ossl had a bug.
• No other JRuby extensions looks to be affected.
• Dated. (joke

For ISO8859_1 JavaString, let's discuss if we met another problem.

Fixed at jruby-openssl 0.7.4. (coming soon)