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

…g of the source file (1.9 edition)
jruby · Aug 24, 2015 · 9cfe533 · 9cfe533
1 parent cb3abd7
commit 9cfe533
Showing 3 changed files with 29 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,6 +36,8 @@
 package org.jruby.parser;
 
 import java.math.BigInteger;
+import java.nio.charset.Charset;
+import java.nio.charset.UnsupportedCharsetException;
 import org.jcodings.Encoding;
 import org.jruby.CompatVersion;
 import org.jruby.RubyBignum;
@@ -1144,6 +1146,22 @@ public void setLexer(RubyYaccLexer lexer) {
     public DStrNode createDStrNode(ISourcePosition position) {
         return new DStrNode(position);
     }
+
+    /**
+     * Used by 1.9 parser (so encoding aware)
+     */
+    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) {
         if (value instanceof StrNode) return new SymbolNode(position, ((StrNode) value).getValue());

diff --git a/core/src/main/java/org/jruby/parser/Ruby19Parser.java b/core/src/main/java/org/jruby/parser/Ruby19Parser.java
@@ -95,7 +95,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.UnnamedRestArgNode;
 import org.jruby.ast.UntilNode;
 import org.jruby.ast.VAliasNode;
@@ -138,7 +137,7 @@ public void setWarnings(IRubyWarnings warnings) {
         support.setWarnings(warnings);
         lexer.setWarnings(warnings);
     }
-					// line 142 "-"
+					// line 141 "-"
   // %token constants
   public static final int kCLASS = 257;
   public static final int kMODULE = 258;
@@ -3553,8 +3552,7 @@ public Object yyparse (RubyYaccLexer yyLex) throws java.io.IOException {
 };
 states[415] = new ParserState() {
   @Override public Object execute(ParserSupport support, RubyYaccLexer lexer, Object yyVal, Object[] yyVals, int yyTop) {
-                    /* FIXME: We may be intern'ing more than once.*/
-  yyVal = new SymbolNode(((Token)yyVals[0+yyTop]).getPosition(), new ByteList(((String)((Token)yyVals[0+yyTop]).getValue()).getBytes(), lexer.getEncoding()));
+                    yyVal = support.asSymbol(lexer.getPosition(), (String) ((Token)yyVals[0+yyTop]).getValue());
     return yyVal;
   }
 };
@@ -3791,11 +3789,11 @@ public Object yyparse (RubyYaccLexer 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(((Token)yyVals[-2+yyTop]).getPosition(), "");
+                         yyVal = support.asSymbol(lexer.getPosition(), "");
                      } else if (((Node)yyVals[-1+yyTop]) instanceof DStrNode) {
                          yyVal = new DSymbolNode(((Token)yyVals[-2+yyTop]).getPosition(), ((DStrNode)yyVals[-1+yyTop]));
                      } else if (((Node)yyVals[-1+yyTop]) instanceof StrNode) {
-                         yyVal = new SymbolNode(((Token)yyVals[-2+yyTop]).getPosition(), ((StrNode)yyVals[-1+yyTop]).getValue());
+                         yyVal = support.asSymbol(((Node)yyVals[-1+yyTop]).getPosition(), ((Node)yyVals[-1+yyTop]));
                      } else {
                          yyVal = new DSymbolNode(((Token)yyVals[-2+yyTop]).getPosition());
                          ((DSymbolNode)yyVal).add(((Node)yyVals[-1+yyTop]));
@@ -4242,7 +4240,7 @@ public Object yyparse (RubyYaccLexer yyLex) throws java.io.IOException {
 states[529] = new ParserState() {
   @Override public Object execute(ParserSupport support, RubyYaccLexer lexer, Object yyVal, Object[] yyVals, int yyTop) {
                     ISourcePosition pos = ((Token)yyVals[-1+yyTop]).getPosition();
-                    yyVal = support.newArrayNode(pos, new SymbolNode(pos, new ByteList(((String) ((Token)yyVals[-1+yyTop]).getValue()).getBytes(), lexer.getEncoding()))).add(((Node)yyVals[0+yyTop]));
+                    yyVal = support.newArrayNode(pos, support.asSymbol(support.getPosition(((Node)yyVals[0+yyTop])), (String) ((Token)yyVals[-1+yyTop]).getValue())).add(((Node)yyVals[0+yyTop]));
     return yyVal;
   }
 };
@@ -4271,7 +4269,7 @@ public Object yyparse (RubyYaccLexer yyLex) throws java.io.IOException {
   }
 };
 }
-					// line 2025 "Ruby19Parser.y"
+					// line 2023 "Ruby19Parser.y"
 
     /** The parse method use an lexer stream and parse it to an AST node 
      * structure
@@ -4290,4 +4288,4 @@ public RubyParserResult parse(ParserConfiguration configuration, LexerSource sou
         return support.getResult();
     }
 }
-					// line 8255 "-"
+					// line 8253 "-"
diff --git a/core/src/main/java/org/jruby/parser/Ruby19Parser.y b/core/src/main/java/org/jruby/parser/Ruby19Parser.y
@@ -92,7 +92,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.UnnamedRestArgNode;
 import org.jruby.ast.UntilNode;
 import org.jruby.ast.VAliasNode;
@@ -1523,8 +1522,7 @@ opt_ensure      : kENSURE compstmt {
 
 literal         : numeric
                 | symbol {
-                    // FIXME: We may be intern'ing more than once.
-  $$ = new SymbolNode($1.getPosition(), new ByteList(((String)$<Token>1.getValue()).getBytes(), lexer.getEncoding()));
+                    $$ = support.asSymbol(lexer.getPosition(), (String) $1.getValue());
                 }
                 | dsym
 
@@ -1693,11 +1691,11 @@ dsym            : tSYMBEG xstring_contents tSTRING_END {
                      // EvStrNode :"#{some expression}"
                      // Ruby 1.9 allows empty strings as symbols
                      if ($2 == null) {
-                         $$ = new SymbolNode($1.getPosition(), "");
+                         $$ = support.asSymbol(lexer.getPosition(), "");
                      } else if ($2 instanceof DStrNode) {
                          $$ = new DSymbolNode($1.getPosition(), $<DStrNode>2);
                      } else if ($2 instanceof StrNode) {
-                         $$ = new SymbolNode($1.getPosition(), $<StrNode>2.getValue());
+                         $$ = support.asSymbol($2.getPosition(), $2);
                      } else {
                          $$ = new DSymbolNode($1.getPosition());
                          $<DSymbolNode>$.add($2);
@@ -1990,7 +1988,7 @@ assoc           : arg_value tASSOC arg_value {
                 }
                 | tLABEL arg_value {
                     ISourcePosition pos = $1.getPosition();
-                    $$ = support.newArrayNode(pos, new SymbolNode(pos, new ByteList(((String) $1.getValue()).getBytes(), lexer.getEncoding()))).add($2);
+                    $$ = support.newArrayNode(pos, support.asSymbol(support.getPosition($2), (String) $1.getValue())).add($2);
                 }
 
 operation       : tIDENTIFIER | tCONSTANT | tFID