This project is archived and is in readonly mode.

#4437 ✓committed
Simon Chiu

[Patch] XML Mini Parser NoMethodError :getc when application running through Phusion Passenger

Reported by Simon Chiu | April 19th, 2010 @ 05:17 AM

Rails Version: 3.0.0 beta 3
Scenario: ActiveResource from Rails App #1 calling to a controller on Rails App #2 with #create action. Rails App #2 was running through Phusion Passenger, and was returning this:

NoMethodError (private method getc' called for<br/>

<PhusionPassenger::Utils::RewindableInput:0x1044e22d0>):

Asked Hongli (thread: http://groups.google.com/group/phusion-passenger/browse_thread/thre...) about this, and apparently the data input stream fed into the Rack app doesn't need to be able to respond to getc.

This error does not occur if App is running through script/server.

Problem: http://github.com/rails/rails/commit/69bc2043f93890048e415dd7c5f1fe...

Here is the patch for your consideration.

Sorry if it's a bit of a hack as this is my first contrib. Thanks - Simon

Comments and changes to this ticket

  • Hongli Lai

    Hongli Lai April 19th, 2010 @ 09:39 AM

    Instead of referring to Phusion Passenger specifically, you should refer to Rack, because Phusion Passenger is just implementing the Rack specification and the restriction is in Rack.

  • Simon Chiu

    Simon Chiu April 19th, 2010 @ 12:50 PM

    Agreed, it should be referred to the rack.input rather than Passenger specifically. Updated the patch to reflect that in the tests. Libxml and Nokogiri parsers have also been updated.

  • Simon Chiu

    Simon Chiu April 21st, 2010 @ 02:26 PM

    • Title changed from “XML Mini Parser NoMethodError :getc when application running through Phusion Passenger” to “[Patch] XML Mini Parser NoMethodError :getc when application running through Phusion Passenger”
  • ieuser

    ieuser April 26th, 2010 @ 06:00 PM

    applied the patch to rails3 beta3 but still get

    PhusionPassenger::Utils::RewindableInput is not a valid input stream. It must walk like either a String, an IO, or a Source.

  • Simon Chiu

    Simon Chiu April 27th, 2010 @ 12:51 PM

    ieuser, Which XML parser backend are you using with your Rails 3 app?

  • Si

    Si April 27th, 2010 @ 04:27 PM

    Hi Simon, I just tried the patch and get the same error:

    PhusionPassenger::Utils::RewindableInput is not a valid input stream.  It must walk like either a String, an IO, or a Source.

    I'm using rexml and here's the backtrace:

    /Users/Si/.rvm/rubies/ruby-1.9.2-head/lib/ruby/1.9.1/rexml/source.rb:22:in `create_from'
/Users/Si/.rvm/rubies/ruby-1.9.2-head/lib/ruby/1.9.1/rexml/parsers/baseparser.rb:146:in `stream='
/Users/Si/.rvm/rubies/ruby-1.9.2-head/lib/ruby/1.9.1/rexml/parsers/baseparser.rb:123:in `initialize'
/Users/Si/.rvm/rubies/ruby-1.9.2-head/lib/ruby/1.9.1/rexml/parsers/treeparser.rb:9:in `new'
/Users/Si/.rvm/rubies/ruby-1.9.2-head/lib/ruby/1.9.1/rexml/parsers/treeparser.rb:9:in `initialize'
/Users/Si/.rvm/rubies/ruby-1.9.2-head/lib/ruby/1.9.1/rexml/document.rb:230:in `new'
/Users/Si/.rvm/rubies/ruby-1.9.2-head/lib/ruby/1.9.1/rexml/document.rb:230:in `build'
/Users/Si/.rvm/rubies/ruby-1.9.2-head/lib/ruby/1.9.1/rexml/document.rb:43:in `initialize'
vendor/rails/activesupport/lib/active_support/xml_mini/rexml.rb:38:in `new'
vendor/rails/activesupport/lib/active_support/xml_mini/rexml.rb:38:in `parse'
vendor/rails/activesupport/lib/active_support/xml_mini.rb:13:in `parse'
  • Jan Kubr

    Jan Kubr April 28th, 2010 @ 10:54 AM

    You'll need to call .read on the received data before passing it to REXML::Document.new. Something like this seems to work:

    def parse(data)
  if !data.respond_to?(:read)
    data = StringIO.new(data || '')
  end

  str = data.gets
  if str.nil?
    {}
  else
    data.rewind
    silence_warnings { require 'rexml/document' } unless defined?(REXML::Document)
    doc = REXML::Document.new(data.read)

    if doc.root
      merge_element!({}, doc.root)
    else
      raise REXML::ParseException,
        "The document #{doc.to_s.inspect} does not have a valid root"
    end
  end
end
  • Simon Chiu

    Simon Chiu April 28th, 2010 @ 12:25 PM

    I wonder if this is one of those incompatibilities with Ruby 1.8 vs 1.9. I was using 1.8.7 when I came across this error, but the patch seemed to have fixed it. I will try and use 1.9-head tonight and see if I can duplicate the problem.

    Just in case, can someone post an example XML which caused the error for them?

  • Si

    Si April 28th, 2010 @ 01:12 PM

    Even something minimal like will do it.

    If I use NokogiriSAX as my XmlMini backend, your patch works as hoped however.

  • Si

    Si April 28th, 2010 @ 01:14 PM

    Ha! I should have used preview :)

    Even something minimal like

    <oops/>

    will do it.

  • Gaël Deest

    Gaël Deest May 4th, 2010 @ 11:11 PM

    Any update on this ? I'm seeing this bug too and it is pretty annoying.

  • Simon Chiu

    Simon Chiu May 5th, 2010 @ 04:46 AM

    Gael, are you also using Ruby 1.9.2-head?

  • Gaël Deest

    Gaël Deest May 5th, 2010 @ 09:00 AM

    Nope, I'm using Ruby Enterprise Edition 1.8.7 with rack 1.1.0 on the remote (ActiveResource) server, behind SSL and HTTP basic authentication. Switching the XmlMini backend from REXML to Nokogiri did not help.

    If I'm understanding the issue correctly, the XML sent by the client can not be parsed because the XML parser expects the rack object to respond to unknown 'getc' and 'ungetc' methods. So this probably affects any XML controller and I would expect more people to complain. Perhaps there is something special in our setup (such as SSL) that triggers the problem ?

  • Simon Chiu

    Simon Chiu May 5th, 2010 @ 11:03 AM

    Gael,

    I'm just curious what is preventing your set up from working.

    Try putting this Gist in your initializer directory and see if it works: http://gist.github.com/373797

    Simon

  • Gaël Deest

    Gaël Deest May 5th, 2010 @ 06:06 PM

    Your monkey patch seems to work. Thanks !

  • Simon Chiu

    Simon Chiu May 5th, 2010 @ 06:37 PM

    Okay, I will look at the patch and see if I messed up somewhere. Thanks Gael.

  • Gaël Deest

    Gaël Deest May 6th, 2010 @ 08:01 AM

    Oh, I had not applied your patch yet so it may be perfectly fine. Thanks to your for taking some time to address this issue !

  • Lawrence Pit

    Lawrence Pit May 6th, 2010 @ 12:46 PM

    • Tag changed from “active_resource, passenger, rack, xml_mini” to “actiondispatch, active_resource, passenger, rack, rails3, xml_mini”
    • Assigned user set to “Jeremy Kemper”

    This shouldn't be solved by patching the xmlmini classes.

    The issue only occurs in development mode. This is because Rack adds the middleware Rack::Lint when in development mode, which results in all input to Rails being wrapped into a Rack::Lint::InputWrapper. This is not an IO class, and therefor fails as input into all the XML parsers.

    Attached is a patch to solve this issue.

    Note: at this moment it's not possible to add a proper test for this, as rack's Rack::MockRequest always returns a StringIO stream for env['rack.input']. I'm currently working on a patch for Rack to remedy this, as I think tests should work with Rack::Lint::InputWrapper instances instead (which would then show the current xml tests failing without the attached patch): StringIO basically doesn't comply with the Rack specification.

  • Mat Schaffer

    Mat Schaffer May 6th, 2010 @ 02:40 PM

    Just thought I'd mention that we've hit this running in production mode as well. Thanks for your work on this, looking forward to a fix.

  • Gaël Deest

    Gaël Deest May 6th, 2010 @ 04:12 PM

    I too met this bug in production.

  • Jeremy Kemper

    Jeremy Kemper May 6th, 2010 @ 06:45 PM

    • State changed from “new” to “open”

    Sounds like our only option for streaming parsing is to provide an IO wrapper than treats getc as read. Nasty.

    Patch + test welcome to read the full IO to a string first instead, reverting the streaming behavior.

  • Lawrence Pit

    Lawrence Pit May 7th, 2010 @ 01:48 AM

    Mat and Gael, it /is/ possible that you get this issue in production as well, because any rack middleware that adds itself and wraps the input which strictly conforms to the Rack protocol will cause this issue.

  • Lawrence Pit

    Lawrence Pit May 7th, 2010 @ 01:49 AM

    Jeremy, attached patch with test.

  • Lawrence Pit

    Lawrence Pit May 7th, 2010 @ 01:55 AM

    • no changes were found...
  • Mat Schaffer

    Mat Schaffer May 7th, 2010 @ 02:43 AM

    Thanks for the clarification, Lawrence. If it helps, the app in question is using Rack::URLMap in config.ru (for switching between the main app and Resque::Server) as well as HoptoadNotifier::Rack in production.rb as middleware.

    I'll try out some of these patches and let you know if I find anything. Thanks!

  • Mat Schaffer

    Mat Schaffer May 7th, 2010 @ 03:34 AM

    That rackinput.diff patch seems to do the trick for my app. Good find, Lawrence!

    I also did a test app without any middleware and was able to produce this under Passenger (development and production envs). Seems that the Rack environment in Passenger is a little more strict than others.

    Thanks again.

  • Repository

    Repository May 7th, 2010 @ 04:59 AM

    • State changed from “open” to “committed”

    (from [1e1d30715ee75312b41045d2af1c68492ea66a05]) Fix parsing xml input by ActionDispatch::ParamsParser

    [#4437 state:committed]

    Signed-off-by: Jeremy Kemper jeremy@bitsweat.net
    http://github.com/rails/rails/commit/1e1d30715ee75312b41045d2af1c68...

  • csnk

    csnk May 18th, 2011 @ 08:22 AM

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

<h2 style="font-size: 14px">Tickets have moved to Github</h2>

The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>

People watching this ticket

Attachments

Tags

Referenced by

Pages