Skip to content

REXML::XPath - problem with 'and' in xpath#1743

Closed
LTe wants to merge 2 commits intorubinius:masterfrom
LTe:rexml_path
Closed

REXML::XPath - problem with 'and' in xpath#1743
LTe wants to merge 2 commits intorubinius:masterfrom
LTe:rexml_path

Conversation

@LTe
Copy link
Copy Markdown
Contributor

@LTe LTe commented May 24, 2012

How to reproduce bug:

require 'rexml/document'
require 'rexml/element'

@body = "<testsuite tests='5' skipped='0' failures='0'>test</testsuite>"
@xpath = "/testsuite[@tests='5' and @skipped='0' and @failures='0']"
@document = REXML::Document.new(@body)
REXML::XPath.match(@document, @xpath)
An exception occurred running rub.rb
    undefined method `inject' on true:TrueClass. (NoMethodError)

Backtrace:
  Kernel(TrueClass)#inject (method_missing) at kernel/delta/kernel.rb:81
                    REXML::XPathParser#expr at /home/lite/.rvm/rubies/rbx-head/lib/19/rexml/xpath_parser.rb:437
             { } in REXML::XPathParser#expr at /home/lite/.rvm/rubies/rbx-head/lib/19/rexml/xpath_parser.rb:302
   { } in Enumerable(Array)#each_with_index at kernel/common/enumerable19.rb:137
                                 Array#each at kernel/bootstrap/array.rb:68
          Enumerable(Array)#each_with_index at kernel/common/enumerable19.rb:136
                    REXML::XPathParser#expr at /home/lite/.rvm/rubies/rbx-head/lib/19/rexml/xpath_parser.rb:296
                   REXML::XPathParser#match at /home/lite/.rvm/rubies/rbx-head/lib/19/rexml/xpath_parser.rb:129
                   REXML::XPathParser#parse at /home/lite/.rvm/rubies/rbx-head/lib/19/rexml/xpath_parser.rb:60
                         REXML::XPath.match at /home/lite/.rvm/rubies/rbx-head/lib/19/rexml/xpath.rb:74
                          Object#__script__ at rub.rb:7
           Rubinius::CodeLoader#load_script at kernel/delta/codeloader.rb:67
           Rubinius::CodeLoader.load_script at kernel/delta/codeloader.rb:109
                    Rubinius::Loader#script at kernel/loader.rb:640
                      Rubinius::Loader#main at kernel/loader.rb:844

When developer uses 'and' many times in xpath then left variable (https://github.com/rubinius/rubinius/blob/master/lib/19/rexml/xpath_parser.rb#L437) is true so inject method will raise exeception.

@travisbot
Copy link
Copy Markdown

travisbot commented May 24, 2012

This pull request passes (merged 59f5aac into 5c924fd).

@dbussink
Copy link
Copy Markdown
Contributor

dbussink commented May 26, 2012

Is this also a problem on MRI? Our stuff in lib/ is a copy of MRI as much as possible. If this is a bug in MRI too and is fixed in newer versions, we should probably just update our stuff in lib/

@LTe
Copy link
Copy Markdown
Contributor Author

LTe commented May 27, 2012

Is this also a problem on MRI?

No (tested on today ruby-head, ruby-1.9.3-p125, ruby-1.8.7-p358)

@dbussink
Copy link
Copy Markdown
Contributor

dbussink commented May 27, 2012

@LTe In that case we should probably import the latest versions from those and not create custom changes in Rubinius, that will only increase the maintenance burden.

@dbussink
Copy link
Copy Markdown
Contributor

dbussink commented May 27, 2012

I also see that you only added a spec for 1.8 mode, any reason that the spec isn't run in 1.9 mode?

@LTe
Copy link
Copy Markdown
Contributor Author

LTe commented May 27, 2012

@LTe In that case we should probably import the latest versions from those and not create custom changes in Rubinius, that will only increase the maintenance burden.

Make sense:
https://github.com/ruby/ruby/blob/trunk/lib/rexml/xpath_parser.rb#L448

I also see that you only added a spec for 1.8 mode, any reason that the spec isn't run in 1.9 mode?

We can delete that (and test this for 2.0 too). My mistake, I wanted to be there "" .. "1.9"

@dbussink
Copy link
Copy Markdown
Contributor

dbussink commented May 27, 2012

If you want to, you can open a pull request where you copy the latest version of REXML from MRI's 1.8.7 and 1.9.3 branch into lib/18/rexml and lib/19/rexml

@LTe
Copy link
Copy Markdown
Contributor Author

LTe commented May 27, 2012

If you want to, you can open a pull request where you copy the latest version of REXML from MRI's 1.8.7 and 1.9.3 branch into lib/18/rexml and lib/19/rexml

With pleasure ;-)

@LTe LTe mentioned this pull request May 27, 2012
Merged
@LTe LTe closed this May 27, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@LTe @travisbot @dbussink