JRuby

[1.9] [RubySpec] Regexp Spec failures with /n

Details

  • Number of attachments :
    3

Description

8)
Regexps with encoding modifiers supports /n (No encoding) with interpolation ERROR
RegexpError: /.../n has a non escaped non ASCII character in non ASCII-8BIT script
/Users/asari/Development/src/jruby/spec/ruby/language/regexp/encoding_spec.rb:89:in `(root)'
org/jruby/RubyBasicObject.java:1753:in `instance_eval'
org/jruby/RubyEnumerable.java:1313:in `all?'
org/jruby/RubyArray.java:1596:in `each'
/Users/asari/Development/src/jruby/spec/ruby/language/regexp/encoding_spec.rb:4:in `(root)'
org/jruby/RubyKernel.java:996:in `load'
/Users/asari/Development/src/jruby/spec/ruby/language/regexp/encoding_spec.rb:56:in `files'
org/jruby/RubyBasicObject.java:1753:in `instance_eval'
org/jruby/RubyArray.java:1596:in `each'

9)
Regexps with encoding modifiers supports /n (No encoding) with interpolation /o ERROR
RegexpError: /.../n has a non escaped non ASCII character in non ASCII-8BIT script
/Users/asari/Development/src/jruby/spec/ruby/language/regexp/encoding_spec.rb:93:in `(root)'
org/jruby/RubyBasicObject.java:1753:in `instance_eval'
org/jruby/RubyEnumerable.java:1313:in `all?'
org/jruby/RubyArray.java:1596:in `each'
/Users/asari/Development/src/jruby/spec/ruby/language/regexp/encoding_spec.rb:4:in `(root)'
org/jruby/RubyKernel.java:996:in `load'
/Users/asari/Development/src/jruby/spec/ruby/language/regexp/encoding_spec.rb:56:in `files'
org/jruby/RubyBasicObject.java:1753:in `instance_eval'
org/jruby/RubyArray.java:1596:in `each'
  1. Text File
    0001-Alternative-fix-for-JRUBY-6143.patch
    03/Dec/11 5:32 AM
    1.0 kB
    Matt Wilbur
  2. Text File
    0001-Fixex-JRUBY-6143.patch
    18/Nov/11 8:11 AM
    1 kB
    Matt Wilbur
  3. Text File
    0002-Fix-small-whitespace-problem.patch
    18/Nov/11 8:12 AM
    0.8 kB
    Matt Wilbur

Issue Links

is duplicated by

Bug - A problem which impairs or prevents the functions of the product. JRUBY-6113 RegExpError during require open-uri in 1.9 mode

  • Blocker - Blocks development and/or testing work, production could not run
  • Closed - The issue is considered finished, the resolution is correct. Issues which are not closed can be reopened.

Bug - A problem which impairs or prevents the functions of the product. JRUBY-6134 [1.9] Regression in DRegexp handling affects open-uri and Bundler

  • Critical - Crashes, loss of data, severe memory leak.
  • Closed - The issue is considered finished, the resolution is correct. Issues which are not closed can be reopened.

Bug - A problem which impairs or prevents the functions of the product. JRUBY-6189 open-uri raises an encoding error in 1.9 mode

  • Major - Major loss of function.
  • Closed - The issue is considered finished, the resolution is correct. Issues which are not closed can be reopened.

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Matt Wilbur added a comment -

I'd like to help with this, though I'm no Java expert. I will try the RubySpec, though a simple script -e'x = /[\r\t\n ]+/n; y=/#

{x}

/n' passed. The failure seems to be related to the encodingNone option on the regex.

Show
Matt Wilbur added a comment - I'd like to help with this, though I'm no Java expert. I will try the RubySpec, though a simple script -e'x = / [\r\t\n ] +/n; y=/# {x} /n' passed. The failure seems to be related to the encodingNone option on the regex.
Hide
Permalink
Matt Wilbur added a comment -

Seems to be related to code getting loaded via a require.
$ echo 'x = /[\r\t\n ]+/n; y=/#

{x}

/n' > x.rb
$ jruby --1.9 x.rb
$ jruby --1.9 -rx -e'puts "Hello"'
RegexpError: /.../n has a non escaped non ASCII character in non ASCII-8BIT script
(root) at /Users/matt/Projects/Ruby/RubySpec/rubyspec/x.rb:1

Show
Matt Wilbur added a comment - Seems to be related to code getting loaded via a require. $ echo 'x = / [\r\t\n ] +/n; y=/# {x} /n' > x.rb $ jruby --1.9 x.rb $ jruby --1.9 -rx -e'puts "Hello"' RegexpError: /.../n has a non escaped non ASCII character in non ASCII-8BIT script (root) at /Users/matt/Projects/Ruby/RubySpec/rubyspec/x.rb:1
Hide
Permalink
Matt Wilbur added a comment -

Believe a relevant discussion is here:
http://www.ruby-forum.com/topic/1053018

Should /a/n.to_str return a String with ASCII-8BIT encoding?

Show
Matt Wilbur added a comment - Believe a relevant discussion is here: http://www.ruby-forum.com/topic/1053018 Should /a/n.to_str return a String with ASCII-8BIT encoding?
Hide
Permalink
Matt Wilbur added a comment -

Spec says

it "returns US_ASCII if the 'n' modifier is supplied and only US-ASCII characters are present" do
/ASCII/n.encoding.should == Encoding::US_ASCII
end

Show
Matt Wilbur added a comment - Spec says it "returns US_ASCII if the 'n' modifier is supplied and only US-ASCII characters are present" do /ASCII/n.encoding.should == Encoding::US_ASCII end
Hide
Permalink
Matt Wilbur added a comment -

I think, perhaps, that the string returned from RubyRegexp.to_s should have flags == StringSupport.CR_7BIT if only US-ASCII characters are present?

Show
Matt Wilbur added a comment - I think, perhaps, that the string returned from RubyRegexp.to_s should have flags == StringSupport.CR_7BIT if only US-ASCII characters are present?
Hide
Permalink
Matt Wilbur added a comment -

The following seems to do the above:

public RubyString(Ruby runtime, RubyClass rubyClass, byte[] value)

{ this(runtime, rubyClass, new ByteList(value)); }

public RubyString(Ruby runtime, RubyClass rubyClass, ByteList value)

{ super(runtime, rubyClass); assert value != null; this.value = value; scanForCodeRange(); }
Show
Matt Wilbur added a comment - The following seems to do the above: public RubyString(Ruby runtime, RubyClass rubyClass, byte[] value) { this(runtime, rubyClass, new ByteList(value)); } public RubyString(Ruby runtime, RubyClass rubyClass, ByteList value) { super(runtime, rubyClass); assert value != null; this.value = value; scanForCodeRange(); }
Hide
Permalink
Matt Wilbur added a comment -

Attached git formatted patches.

Show
Matt Wilbur added a comment - Attached git formatted patches.
Hide
Permalink
Hiro Asari added a comment -

Thank you, Matt.

I merged your patch to the master: 1cf1d8c

Show
Hiro Asari added a comment - Thank you, Matt. I merged your patch to the master: 1cf1d8c
Hide
Permalink
Thomas E Enebo added a comment -

1cf1d8c1ed069fc3833b9750abd6af7d7d042a83 should be solved differently (please revert). That patch just requires almost every string in 1.8 and 1.9 to be walked once during creation to determine if all characters are more than one byte or not. I am pretty sure this will have a significant impact on string performance.

Ultimately, anything which wants to work with a string in 1.9 mode should hit logic which recognizes we have not determined coderange yet and then do the costly walk. Ultimately, we a missing some of that logic somewhere (coderange is a brittle icky thing, but we followed MRI's lead on this).

I really really wish we had nice isolation between 1.8 and 1.9 code paths too

Show
Thomas E Enebo added a comment - 1cf1d8c1ed069fc3833b9750abd6af7d7d042a83 should be solved differently (please revert). That patch just requires almost every string in 1.8 and 1.9 to be walked once during creation to determine if all characters are more than one byte or not. I am pretty sure this will have a significant impact on string performance. Ultimately, anything which wants to work with a string in 1.9 mode should hit logic which recognizes we have not determined coderange yet and then do the costly walk. Ultimately, we a missing some of that logic somewhere (coderange is a brittle icky thing, but we followed MRI's lead on this). I really really wish we had nice isolation between 1.8 and 1.9 code paths too
Hide
Permalink
Matt Wilbur added a comment -

This patch places the code range scan at the latest possible moment to fix this bug.

Show
Matt Wilbur added a comment - This patch places the code range scan at the latest possible moment to fix this bug.
Hide
Permalink
Hiro Asari added a comment -

Tom,

Can you review Matt's patch from Dec 3? I confirmed that it works as advertised.

Show
Hiro Asari added a comment - Tom, Can you review Matt's patch from Dec 3? I confirmed that it works as advertised.
Hide
Permalink
Thomas E Enebo added a comment -

I fixed this very very very slightly differently last week on master and 1.6 branch. Matt's patch was a great help...Unfortunately, both our patches are not the ideal patch, but it is better to get it working at this point

Show
Thomas E Enebo added a comment - I fixed this very very very slightly differently last week on master and 1.6 branch. Matt's patch was a great help...Unfortunately, both our patches are not the ideal patch, but it is better to get it working at this point

People

  • Assignee:
    Thomas E Enebo
    Reporter:
    Hiro Asari
Vote (0)
Watch (3)

Dates

  • Created:
    Updated:
    Resolved: