Fixes #1328. Encoding of symbol literals does not respect the encodin…

…g of the source file
jruby · Aug 21, 2015 · e590e5d · e590e5d
1 parent 43da800
commit e590e5d
Showing 3 changed files with 27 additions and 15 deletions.
diff --git a/core/src/main/java/org/jruby/parser/ParserSupport.java b/core/src/main/java/org/jruby/parser/ParserSupport.java
@@ -36,10 +36,13 @@
 package org.jruby.parser;
 
 import java.math.BigInteger;
+import java.nio.charset.Charset;
+import java.nio.charset.UnsupportedCharsetException;
 import java.util.ArrayList;
 import java.util.List;
 import org.jcodings.Encoding;
 import org.jruby.RubyBignum;
+import org.jruby.RubyEncoding;
 import org.jruby.RubyRegexp;
 import org.jruby.ast.*;
 import org.jruby.ast.types.ILiteralNode;
@@ -879,6 +882,19 @@ public void setLexer(RubyLexer lexer) {
     public DStrNode createDStrNode(ISourcePosition position) {
         return new DStrNode(position, lexer.getEncoding());
     }
+
+    public Node asSymbol(ISourcePosition position, String value) {
+        // FIXME: tLABEL and identifiers could return ByteList and not String and make String on-demand for method names
+        // or lvars.  This would prevent this re-extraction of bytes from a string with proper charset
+        try {
+            Charset charset = lexer.getEncoding().getCharset();
+            if (charset != null) return new SymbolNode(position, new ByteList(value.getBytes(charset), lexer.getEncoding()));
+        } catch (UnsupportedCharsetException e) {}
+
+        // for non-charsets we are screwed here since bytes will file.encoding and not what we read them as (see above FIXME for
+        // a much more invasive solution.
+        return new SymbolNode(position, new ByteList(value.getBytes(), lexer.getEncoding()));
+    }
 
     public Node asSymbol(ISourcePosition position, Node value) {
         return value instanceof StrNode ? new SymbolNode(position, ((StrNode) value).getValue()) :

diff --git a/core/src/main/java/org/jruby/parser/RubyParser.java b/core/src/main/java/org/jruby/parser/RubyParser.java
@@ -34,7 +34,6 @@
 import java.io.IOException;
 
 import org.jruby.ast.ArgsNode;
-import org.jruby.ast.ArgumentNode;
 import org.jruby.ast.ArrayNode;
 import org.jruby.ast.AssignableNode;
 import org.jruby.ast.BackRefNode;
@@ -103,7 +102,6 @@
 import org.jruby.ast.SelfNode;
 import org.jruby.ast.StarNode;
 import org.jruby.ast.StrNode;
-import org.jruby.ast.SymbolNode;
 import org.jruby.ast.TrueNode;
 import org.jruby.ast.UnnamedRestArgNode;
 import org.jruby.ast.UntilNode;
@@ -147,7 +145,7 @@ public void setWarnings(IRubyWarnings warnings) {
         support.setWarnings(warnings);
         lexer.setWarnings(warnings);
     }
-					// line 151 "-"
+					// line 149 "-"
   // %token constants
   public static final int kCLASS = 257;
   public static final int kMODULE = 258;
@@ -4270,7 +4268,7 @@ public Object yyparse (RubyLexer yyLex) throws java.io.IOException {
 };
 states[454] = new ParserState() {
   @Override public Object execute(ParserSupport support, RubyLexer lexer, Object yyVal, Object[] yyVals, int yyTop) {
-                    yyVal = new SymbolNode(lexer.getPosition(), new ByteList(((String)yyVals[0+yyTop]).getBytes(), lexer.getEncoding()));
+                    yyVal = support.asSymbol(lexer.getPosition(), ((String)yyVals[0+yyTop]));
     return yyVal;
   }
 };
@@ -4580,11 +4578,11 @@ public Object yyparse (RubyLexer yyLex) throws java.io.IOException {
                      /* EvStrNode :"#{some expression}"*/
                      /* Ruby 1.9 allows empty strings as symbols*/
                      if (((Node)yyVals[-1+yyTop]) == null) {
-                         yyVal = new SymbolNode(lexer.getPosition(), new ByteList(new byte[0], lexer.getEncoding()));
+                         yyVal = support.asSymbol(lexer.getPosition(), "");
                      } else if (((Node)yyVals[-1+yyTop]) instanceof DStrNode) {
                          yyVal = new DSymbolNode(((Node)yyVals[-1+yyTop]).getPosition(), ((DStrNode)yyVals[-1+yyTop]));
                      } else if (((Node)yyVals[-1+yyTop]) instanceof StrNode) {
-                         yyVal = new SymbolNode(((Node)yyVals[-1+yyTop]).getPosition(), ((StrNode)yyVals[-1+yyTop]).getValue());
+                         yyVal = support.asSymbol(((Node)yyVals[-1+yyTop]).getPosition(), ((Node)yyVals[-1+yyTop]));
                      } else {
                          yyVal = new DSymbolNode(((Node)yyVals[-1+yyTop]).getPosition());
                          ((DSymbolNode)yyVal).add(((Node)yyVals[-1+yyTop]));
@@ -5250,7 +5248,7 @@ public Object yyparse (RubyLexer yyLex) throws java.io.IOException {
 };
 states[611] = new ParserState() {
   @Override public Object execute(ParserSupport support, RubyLexer lexer, Object yyVal, Object[] yyVals, int yyTop) {
-                    SymbolNode label = new SymbolNode(support.getPosition(((Node)yyVals[0+yyTop])), new ByteList(((String)yyVals[-1+yyTop]).getBytes(), lexer.getEncoding()));
+                    Node label = support.asSymbol(support.getPosition(((Node)yyVals[0+yyTop])), ((String)yyVals[-1+yyTop]));
                     yyVal = new KeyValuePair<Node,Node>(label, ((Node)yyVals[0+yyTop]));
     return yyVal;
   }
@@ -5301,7 +5299,7 @@ public Object yyparse (RubyLexer yyLex) throws java.io.IOException {
   }
 };
 }
-					// line 2533 "RubyParser.y"
+					// line 2531 "RubyParser.y"
 
     /** The parse method use an lexer stream and parse it to an AST node 
      * structure
@@ -5316,4 +5314,4 @@ public RubyParserResult parse(ParserConfiguration configuration) throws IOExcept
         return support.getResult();
     }
 }
-					// line 9885 "-"
+					// line 9883 "-"
diff --git a/core/src/main/java/org/jruby/parser/RubyParser.y b/core/src/main/java/org/jruby/parser/RubyParser.y
@@ -40,7 +40,6 @@ import org.jruby.ast.BlockArgNode;
 import org.jruby.ast.BlockNode;
 import org.jruby.ast.BlockPassNode;
 import org.jruby.ast.BreakNode;
-import org.jruby.ast.CallNode;
 import org.jruby.ast.ClassNode;
 import org.jruby.ast.ClassVarNode;
 import org.jruby.ast.ClassVarAsgnNode;
@@ -99,7 +98,6 @@ import org.jruby.ast.SClassNode;
 import org.jruby.ast.SelfNode;
 import org.jruby.ast.StarNode;
 import org.jruby.ast.StrNode;
-import org.jruby.ast.SymbolNode;
 import org.jruby.ast.TrueNode;
 import org.jruby.ast.UnnamedRestArgNode;
 import org.jruby.ast.UntilNode;
@@ -1864,7 +1862,7 @@ literal         : numeric {
                     $$ = $1;
                 }
                 | symbol {
-                    $$ = new SymbolNode(lexer.getPosition(), new ByteList($1.getBytes(), lexer.getEncoding()));
+                    $$ = support.asSymbol(lexer.getPosition(), $1);
                 }
                 | dsym
 
@@ -2066,11 +2064,11 @@ dsym            : tSYMBEG xstring_contents tSTRING_END {
                      // EvStrNode :"#{some expression}"
                      // Ruby 1.9 allows empty strings as symbols
                      if ($2 == null) {
-                         $$ = new SymbolNode(lexer.getPosition(), new ByteList(new byte[0], lexer.getEncoding()));
+                         $$ = support.asSymbol(lexer.getPosition(), "");
                      } else if ($2 instanceof DStrNode) {
                          $$ = new DSymbolNode($2.getPosition(), $<DStrNode>2);
                      } else if ($2 instanceof StrNode) {
-                         $$ = new SymbolNode($2.getPosition(), $<StrNode>2.getValue());
+                         $$ = support.asSymbol($2.getPosition(), $2);
                      } else {
                          $$ = new DSymbolNode($2.getPosition());
                          $<DSymbolNode>$.add($2);
@@ -2480,7 +2478,7 @@ assoc           : arg_value tASSOC arg_value {
                     $$ = new KeyValuePair<Node,Node>($1, $3);
                 }
                 | tLABEL arg_value {
-                    SymbolNode label = new SymbolNode(support.getPosition($2), new ByteList($1.getBytes(), lexer.getEncoding()));
+                    Node label = support.asSymbol(support.getPosition($2), $1);
                     $$ = new KeyValuePair<Node,Node>(label, $2);
                 }
                 | tSTRING_BEG string_contents tLABEL_END arg_value {