ncr / rack-proxy

A request/response rewriting HTTP proxy. A Rack app.
MIT License
269 stars 94 forks source link

Using Rack::Proxy as a middleware instead of using it as a standard Ruby class #55

Closed bogdanRada closed 8 years ago

bogdanRada commented 8 years ago

In order for Rack Proxy to work as middleware for a Rack application (Rails or sinatra or any rack based application ) the initialize method for the Rack::Proxy class needed to be changed to receive first the application and then the options.

why is it useful? Consider folowing scenario: Having a application that uses by default Savon for doing request to a web-service, but want to proxy only some requests to another server. could be easily achieved doing this:

require 'rack/proxy'
class RackPhpProxy < Rack::Proxy

  def perform_request(env)
    request = Rack::Request.new(env)
    if request.path =~ %r{\.php}
      env["HTTP_HOST"] = "localhost"
      env["REQUEST_PATH"] = "/php/#{request.fullpath}"
      super(env)
    else
      @app.call(env)
    end
  end
end

This means in Rails you could use this like this:

Rails.application.config.middleware.use RackPhpProxy, {ssl_verify_none: true}

and in sintra you could do like this:

class MyAwesomeSinatra < Sinatra::Base
   use  RackPhpProxy, {ssl_verify_none: true}
end

instead of doing

RackPhpProxy.new(ssl_verify_none: true)

In this example only requests that end with ".php" are proxied , the other requests are not proxied.

This might be a backward incompatible change and the README might need updated too. I am opened though to suggestions.

This fixes also #53

Let me know what you think. Thank you very much

ncr commented 8 years ago

This idea seems sensible. It would have to be backward compatible though, and docs and tests should be updated too. I think this change would fully deserve a new minor version: 0.6.0 :)

bogdanRada commented 8 years ago

i can work on tests, but it seems rack-test is not compatible with ruby > 2.1.5 . Would you mind if i rewrite tests to use rspec instead? or maybe minitest? which seems to be compatible .

You can see this issue here: https://travis-ci.org/bogdanRada/rack-proxy

ncr commented 8 years ago

I'm running Ruby 2.3.0 and the tests run fine (except one is failing, but this is not related):

/dev/rack-proxy (master)$ ruby -v

ruby 2.3.0p0 (2015-12-25 revision 53290) [x86_64-darwin15]

~/dev/rack-proxy (master)$ rake test

/Users/ncr/.rubies/ruby-2.3.0/bin/ruby -I"lib:test" -I"/Users/ncr/.gem/ruby/2.3.0/gems/rake-10.5.0/lib" "/Users/ncr/.gem/ruby/2.3.0/gems/rake-10.5.0/lib/rake/rake_test_loader.rb" "test/http_streaming_response_test.rb" "test/net_http_hacked_test.rb" "test/rack_proxy_test.rb"

Loaded suite /Users/ncr/.gem/ruby/2.3.0/gems/rake-10.5.0/lib/rake/rake_test_loader

Started

_........._F

Failure:

Google always sets a cookie, yo. Where my cookies at?!.

is not true. test_http_full_request_headers(RackProxyTest) /Users/ncr/dev/rack-proxy/test/rack_proxy_test.rb:35:in `test_http_full_request_headers' ``` 32: app(:streaming => false) 33: app.host = 'www.google.com' 34: get "/" ``` - => 35: assert !Array(last_response['Set-Cookie']).empty?, 'Google always sets a cookie, yo. Where my cookies at?!'* 36: end 37: 38: def test_https_streaming # _......_ Finished in 3.767812 seconds. --- 16 tests, 43 assertions, 1 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications 93.75% passed --- 4.25 tests/s, 11.41 assertions/s rake aborted! Command failed with status (1): [ruby -I"lib:test" -I"/Users/ncr/.gem/ruby/2.3.0/gems/rake-10.5.0/lib" "/Users/ncr/.gem/ruby/2.3.0/gems/rake-10.5.0/lib/rake/rake_test_loader.rb" "test/http_streaming_response_test.rb" "test/net_http_hacked_test.rb" "test/rack_proxy_test.rb" ] czw., 19.05.2016 o 12:56 użytkownik rada bogdan raul < notifications@github.com> napisał: > i can work on tests, but it seems rack-test is not compatible with ruby > > 2.1.5 . Would you mind if i rewrite tests to use rspec instead? or maybe > minitest? which seems to be compatible . > > — > You are receiving this because you commented. > Reply to this email directly or view it on GitHub > https://github.com/ncr/rack-proxy/pull/55#issuecomment-220291931
bogdanRada commented 8 years ago

I am really nto sure how you're doing that. I get this error: cannot load such file -- test/unit (LoadError) . Do you have something locally installed?

bogdanRada commented 8 years ago

I was able to fix it by adding to gemspec this line:

s.add_development_dependency("test-unit")

I think this should be included in the gem , otherwise people would have hard time running tests Now the build is passing: https://travis-ci.org/bogdanRada/rack-proxy .

I commited the changes in this pull request for that. Hope that is fine.

The tests for this branch are passing https://travis-ci.org/bogdanRada/rack-proxy/builds/131397008 . So i don;t think we need to update tests.

ncr commented 8 years ago

Looks ok.

czw., 19.05.2016 o 14:03 użytkownik rada bogdan raul < notifications@github.com> napisał:

I was able to fix it by adding to gemspec this line:

s.add_development_dependency("test-unit")

— You are receiving this because you commented. Reply to this email directly or view it on GitHub https://github.com/ncr/rack-proxy/pull/55#issuecomment-220304371

bogdanRada commented 8 years ago

updated the readme too. I think now the only change is to bump the version, but that i think should be done after merging the pull request by the owner or one of the collaborators. Let me know if it looks ok, Thank you very much

bogdanRada commented 8 years ago

updated the code, Build is passing: https://travis-ci.org/bogdanRada/rack-proxy/builds/131414275

bogdanRada commented 8 years ago

Build is still passing https://travis-ci.org/bogdanRada/rack-proxy/builds/131415945

bogdanRada commented 8 years ago

Applied suggestion for conditional . Build is passing :https://travis-ci.org/bogdanRada/rack-proxy/builds/131432384

ncr commented 8 years ago

Thanks a lot! Will release a new gem shortly.

bogdanRada commented 8 years ago

Awesome . thanks