Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

unable to set REMOTE_ADDR in rack env #50

Closed
pivotalbonobos opened this issue Sep 14, 2012 · 4 comments
Closed

unable to set REMOTE_ADDR in rack env #50

pivotalbonobos opened this issue Sep 14, 2012 · 4 comments

Comments

@pivotalbonobos
Copy link

I'm trying to set the HTTP_REMOTE_ADDR header to test accessing the API as a remote user. This worked fine with RSpec request specs, but broke upon converting to rspec_api_documentation.

Looking at the request.env that the controller gets, I saw that REMOTE_ADDR was "127.0.0.1", while HTTP_REMOTE_ADDR was the desired value. I eventually traced the problem back to Rack::Test::Session#default_env, which sets REMOTE_ADDR, and RspecApiDocumentation::Headers#headers_to_env which make it impossible to set REMOTE_ADDR.

I'm not entirely sure that it's rspec_api_documentation's responsibility to override REMOTE_ADDR, but it seems like the most plausible possibility since other test libraries use Rack::Test without this problem.

The following monkey patch works around the issue for me:

module RspecApiDocumentation::Headers
  def headers_to_env_with_remote_addr_fix headers
    env = headers_to_env_without_remote_addr_fix(headers)
    env["REMOTE_ADDR"] = env["HTTP_REMOTE_ADDR"] if env["HTTP_REMOTE_ADDR"]
    env
  end
  alias_method_chain :headers_to_env, :remote_addr_fix
end
@samwgoldman
Copy link
Contributor

How are you setting the header? I suspect you are setting headers in the CGI style (HTTP_*, CONTENT_LENGTH, etc) and this testing library is designed to be more like HTTP, so you would want to set "Remote-Addr".

Honestly I'm not familiar with REMOTE_ADDR, but as far as I can tell it isn't a standard HTTP header and wouldn't be sent with a request. Can you share your use case and maybe clue me in on the purpose of setting REMOTE_ADDR?

@pivotalbonobos
Copy link
Author

Here's an edited version of my spec:

resource "User", type: :api do
  get "/api/v1/users/:id.json" do
    let(:id) { 1 }
    header "Remote-Addr", ip_from_finland

    example_request 'get a user by id' do
      status.should == 200
      json_response['id'].should be_present
      json_response['current_country_code'].should == "FI"
    end
  end
end

It's not a casing issue, since the header does get correctly delivered to the Rails controller, it's just not integrated with the Rack::Test env correctly.

REMOTE_ADDR is set by HTTP (eg. Apache/Nginx) proxies to identify the requesting client. Rails uses REMOTE_ADDR in combination with X-Forwarded-For and some other headers to provide the IP address of the end user.

@samwgoldman
Copy link
Contributor

Thanks for the clarification.

Personally, I don't think header "Remote-Addr", ip_from_finland makes much sense because Remote-Addr isn't a HTTP header. It's a CGI environment variable that the server populates based on the request.

In my mind, RspecApiDocumentation examples are from the perspective of the end-user or client, issuing a HTTP request. Rack::Test examples are from the perspective of a web server, sending requests to a Rack application. Does that make sense?

@oestrich Let me know what you think about this.

If RspecApiDocumentation is supposed to be more aligned with Rack::Test, then we should assume a CGI environment and expose direct access to it.

If RspecApiDocumentation is supposed to simulate a client, then we should let the example set the client's IP address, and somehow translate that into the REMOTE_ADDR environment variable for RackTestClient and friends.

@oestrich
Copy link
Contributor

oestrich commented May 3, 2013

Sorry about taking a while to get back to this.

If Remote-Addr isn't a HTTP header I don't think we should support it via the DSL. I can get behind adding setting an IP address otherwise though.

Maybe something similar to:

resource "User", type: :api do
  get "/api/v1/users/:id.json" do
    let(:id) { 1 }

    simulated_ip { ip_from_finland }

    example_request 'get a user by id' do
      status.should == 200
      json_response['id'].should be_present
      json_response['current_country_code'].should == "FI"
    end
  end
end

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants