singlebrook / utf8-cleaner

MIT License
277 stars 44 forks source link

body sanitization #13

Closed parallel588 closed 10 years ago

parallel588 commented 10 years ago

Hi guys, I'm checking last version (0.0.7) and I'm receiving the following error:

==> /var/log/passenger.log <==
App 27370 stderr: [ 2014-04-09 06:50:27.2568 27714/0x00000006110868(Worker 1) utils.rb:68 ]: *** Exception NoMethodError in Rack application object (undefined method `reopen' for #<PhusionPassenger::Utils::TeeInput:0x000000075b32c0>) (process 27714, thread 0x00000006110868(Worker 1)):
App 27370 stderr:   from /home/deploy/luxfix/shared/bundle/ruby/1.9.1/gems/utf8-cleaner-0.0.7/lib/utf8-cleaner/middleware.rb:40:in `sanitize_env_rack_input'
App 27370 stderr:   from /home/deploy/luxfix/shared/bundle/ruby/1.9.1/gems/utf8-cleaner-0.0.7/lib/utf8-cleaner/middleware.rb:25:in `sanitize_env'
App 27370 stderr:   from /home/deploy/luxfix/shared/bundle/ruby/1.9.1/gems/utf8-cleaner-0.0.7/lib/utf8-cleaner/middleware.rb:18:in `call'
App 27370 stderr:   from /home/deploy/luxfix/shared/bundle/ruby/1.9.1/gems/railties-3.2.17/lib/rails/engine.rb:484:in `call'
App 27370 stderr:   from /home/deploy/luxfix/shared/bundle/ruby/1.9.1/gems/railties-3.2.17/lib/rails/application.rb:231:in `call'
App 27370 stderr:   from /home/deploy/luxfix/shared/bundle/ruby/1.9.1/gems/railties-3.2.17/lib/rails/railtie/configurable.rb:30:in `method_missing'
App 27370 stderr:   from /home/deploy/.rvm/gems/ruby-1.9.3-p484/gems/passenger-4.0.41/lib/phusion_passenger/rack/thread_handler_extension.rb:74:in `process_request'
App 27370 stderr:   from /home/deploy/.rvm/gems/ruby-1.9.3-p484/gems/passenger-4.0.41/lib/phusion_passenger/request_handler/thread_handler.rb:141:in `accept_and_process_next_request'
App 27370 stderr:   from /home/deploy/.rvm/gems/ruby-1.9.3-p484/gems/passenger-4.0.41/lib/phusion_passenger/request_handler/thread_handler.rb:109:in `main_loop'
App 27370 stderr:   from /home/deploy/.rvm/gems/ruby-1.9.3-p484/gems/passenger-4.0.41/lib/phusion_passenger/request_handler.rb:448:in `block (3 levels) in start_threads'
App 27370 stderr: [ 2014-04-09 06:50:27.2570 27714/0x00000006110868(Worker 1) request_handler/thread_handler.rb:153 ]: Request done.
juno commented 10 years ago

Same here with Unicorn 4.8.2.

E, [2014-04-09T09:03:41.869236 #5] ERROR -- : app error: undefined method `reopen' for #<Unicorn::TeeInput:0x007fc9c435f350> (NoMethodError) Heroku/some-app
E, [2014-04-09T09:03:41.869602 #5] ERROR -- : /app/vendor/bundle/ruby/2.1.0/gems/utf8-cleaner-0.0.7/lib/utf8-cleaner/middleware.rb:40:in `sanitize_env_rack_input'
E, [2014-04-09T09:03:41.869842 #5] ERROR -- : /app/vendor/bundle/ruby/2.1.0/gems/utf8-cleaner-0.0.7/lib/utf8-cleaner/middleware.rb:25:in `sanitize_env'
E, [2014-04-09T09:03:41.870068 #5] ERROR -- : /app/vendor/bundle/ruby/2.1.0/gems/utf8-cleaner-0.0.7/lib/utf8-cleaner/middleware.rb:18:in `call'
E, [2014-04-09T09:03:41.870220 #5] ERROR -- : /app/vendor/bundle/ruby/2.1.0/gems/railties-4.0.4/lib/rails/engine.rb:511:in `call'
E, [2014-04-09T09:03:41.870350 #5] ERROR -- : /app/vendor/bundle/ruby/2.1.0/gems/railties-4.0.4/lib/rails/application.rb:97:in `call'
E, [2014-04-09T09:03:41.869602 #5] ERROR -- : /app/vendor/bundle/ruby/2.1.0/gems/utf8-cleaner-0.0.7/lib/utf8-cleaner/middleware.rb:40:in `sanitize_env_rack_input' Heroku/some-app Context
E, [2014-04-09T09:03:41.869842 #5] ERROR -- : /app/vendor/bundle/ruby/2.1.0/gems/utf8-cleaner-0.0.7/lib/utf8-cleaner/middleware.rb:25:in `sanitize_env' Heroku/some-app Context
E, [2014-04-09T09:03:41.870068 #5] ERROR -- : /app/vendor/bundle/ruby/2.1.0/gems/utf8-cleaner-0.0.7/lib/utf8-cleaner/middleware.rb:18:in `call' Heroku/some-app Context
E, [2014-04-09T09:03:41.870220 #5] ERROR -- : /app/vendor/bundle/ruby/2.1.0/gems/railties-4.0.4/lib/rails/engine.rb:511:in `call' Heroku/some-app Context
E, [2014-04-09T09:03:41.870350 #5] ERROR -- : /app/vendor/bundle/ruby/2.1.0/gems/railties-4.0.4/lib/rails/application.rb:97:in `call' Heroku/some-app Context
E, [2014-04-09T09:03:41.870477 #5] ERROR -- : /app/vendor/bundle/ruby/2.1.0/gems/railties-4.0.4/lib/rails/railtie/configurable.rb:30:in `method_missing' Heroku/some-app Context

This problem introduced by 893fadb50b17b540a215411b0391df90ea3cda5c. That commit assumes that env['rack.input'] has interface like StringIO. But I think that we should it treat as a Rack::Lint::InputWrapper.

accessd commented 10 years ago

same issue

sbleon commented 10 years ago

Sorry about the error, folks! Sticking to v0.0.6 is probably a good idea for now.

From the Rack docs http://rack.rubyforge.org/doc/SPEC.html: "The input stream must respond to gets, each, read and rewind." reopen is not in that list. It's not clear how we go about modifying the input stream in other Rack servers. Any ideas?

Tagging @salrepe, the original author.

On Wed, Apr 9, 2014 at 8:32 AM, Andrey Morskov notifications@github.comwrote:

same issue

Reply to this email directly or view it on GitHubhttps://github.com/singlebrook/utf8-cleaner/issues/13#issuecomment-39958036 .

salrepe commented 10 years ago

Uops, you are right, maybe as @juno comments, Rack::Lint::InputWrapper can be used to wrapper it. Server wrappers are based on that.

def sanitize_env_rack_input(env)
  if value = env['rack.input'].read
    cleaned_value = cleaned_uri_string(value)
    env['rack.input'] = Rack::Lint::InputWrapper.new(StringIO.new(cleaned_value)) if cleaned_value
  end
  env['rack.input'].rewind
end

The fact is that, with this solution, the wrapper class will be changed, but it would respond to gets, each, read and rewind. What do you think?

accessd commented 10 years ago

@salrepe I think can do without Rack::Lint::InputWrapper and use only StringIO

salrepe commented 10 years ago

Yes, it works, but with Rack::Lint::InputWrapper we maintain what rack docs points http://rack.rubyforge.org/doc/SPEC.html: "The input stream must respond to gets, each, read and rewind.

What do you think?

juno commented 10 years ago

Sorry for the confusion. I just wanted to say that we can expect rack.input has methods defined in Rack::Lint::InputWrapper, but not required to use or wrap with it.

@salrepe your solution is reasonable and worked with Unicorn. :smiley: Maybe it's not required to wrap within Rack::Lint::InputWrapper.

sbleon commented 10 years ago

Can anyone else confirm that #14 works for them? If so, I'll merge it.

juno commented 10 years ago

14 worked for me with Unicorn.