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

bogdanRada · 2016-05-18T08:10:32Z

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

… proxy

ncr · 2016-05-19T10:13:38Z

lib/rack/proxy.rb

@@ -39,7 +39,8 @@ def reconstruct_header_name(name)
    end

    # @option opts [String, URI::HTTP] :backend Backend host to proxy requests to
-    def initialize(opts = {})
+    def initialize(app = nil, opts= {})


This would have to be backward compatible: if the first arg is a hash, then it should be assigned to the opts variable.

ok, just wanted to make sure. Thank you

Still need that conditional check for the app param being a hash :)

added the conditional

ncr · 2016-05-19T10:18:42Z

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 · 2016-05-19T10:56:23Z

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 · 2016-05-19T11:28:53Z

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
#55 (comment)

bogdanRada · 2016-05-19T11:40:17Z

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 · 2016-05-19T12:03:19Z

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 · 2016-05-19T12:12:05Z

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
#55 (comment)

bogdanRada · 2016-05-19T12:23:42Z

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

ncr · 2016-05-19T13:08:02Z

lib/rack/proxy.rb

@@ -105,6 +106,7 @@ def perform_request(env)
        http.read_timeout = read_timeout
        http.verify_mode = OpenSSL::SSL::VERIFY_NONE if use_ssl && ssl_verify_none
        http.ssl_version = @ssl_version if @ssl_version
+        http.set_debug_output($stdout)


This should be removed.

Fixed . Added that by mistake. I was using it to debug the proxy requests locally

bogdanRada · 2016-05-19T13:23:57Z

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

bogdanRada · 2016-05-19T13:31:36Z

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

ncr · 2016-05-19T13:42:04Z

lib/rack/proxy.rb

-    def initialize(opts = {})
+    def initialize(app = nil, opts= {})
+      @app = app
+      opts = app.is_a?(Hash) ? app.merge(opts) : opts


Not sure why app.merge(opts). In case the old way of calling things is used, this has no effect I think.

I would write it like this:

if app.is_a?(Hash) opts = app @app = nil else @app = app end

This way, we also avoid having incorrectly set @app.

bogdanRada · 2016-05-19T14:34:14Z

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

ncr · 2016-05-19T15:11:47Z

Thanks a lot! Will release a new gem shortly.

bogdanRada · 2016-05-19T17:48:12Z

Awesome . thanks

using rack proxy as a middleware - updated initialize method for Rack…

0c81886

… proxy

bogdanRada mentioned this pull request May 18, 2016

undefined method `fetch' for #<NewRelic::Rack::DeveloperMode> #53

Closed

ncr reviewed May 19, 2016
View reviewed changes

bogdanRada added 3 commits May 19, 2016 13:42

travis check

57f4e0c

travis start

6a9ea0f

trying to fix tests on ruby > 2.1.5

2505e5b

override the script for travis- hopefully now works

9c7cedd

test-unit integration

63456fb

bogdanRada added 2 commits May 19, 2016 15:18

updated Readme

a990be2

Merge branch 'feature/readme'

4df1800

ncr reviewed May 19, 2016
View reviewed changes

check hash options

ca7b761

update conditional

3773fdd

ncr reviewed May 19, 2016
View reviewed changes

apply suggestion for conditional

0f5f796

ncr closed this May 19, 2016

ncr reopened this May 19, 2016

ncr merged commit 06243d5 into ncr:master May 19, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

bogdanRada commented May 18, 2016 •

edited

Loading

ncr May 19, 2016

bogdanRada May 19, 2016

ncr May 19, 2016

bogdanRada May 19, 2016

ncr commented May 19, 2016

bogdanRada commented May 19, 2016 •

edited

Loading

ncr commented May 19, 2016

bogdanRada commented May 19, 2016

bogdanRada commented May 19, 2016 •

edited

Loading

ncr commented May 19, 2016

bogdanRada commented May 19, 2016

ncr May 19, 2016

bogdanRada May 19, 2016

bogdanRada commented May 19, 2016

bogdanRada commented May 19, 2016

ncr May 19, 2016

bogdanRada commented May 19, 2016

ncr commented May 19, 2016

bogdanRada commented May 19, 2016

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

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

Conversation

bogdanRada commented May 18, 2016 • edited Loading

ncr May 19, 2016

Choose a reason for hiding this comment

bogdanRada May 19, 2016

Choose a reason for hiding this comment

ncr May 19, 2016

Choose a reason for hiding this comment

bogdanRada May 19, 2016

Choose a reason for hiding this comment

ncr commented May 19, 2016

bogdanRada commented May 19, 2016 • edited Loading

ncr commented May 19, 2016

bogdanRada commented May 19, 2016

bogdanRada commented May 19, 2016 • edited Loading

ncr commented May 19, 2016

bogdanRada commented May 19, 2016

ncr May 19, 2016

Choose a reason for hiding this comment

bogdanRada May 19, 2016

Choose a reason for hiding this comment

bogdanRada commented May 19, 2016

bogdanRada commented May 19, 2016

ncr May 19, 2016

Choose a reason for hiding this comment

bogdanRada commented May 19, 2016

ncr commented May 19, 2016

bogdanRada commented May 19, 2016

bogdanRada commented May 18, 2016 •

edited

Loading

bogdanRada commented May 19, 2016 •

edited

Loading

bogdanRada commented May 19, 2016 •

edited

Loading