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

Proxy Support #153

Closed
wants to merge 5 commits into from
Closed

Proxy Support #153

wants to merge 5 commits into from

Conversation

Asmod4n
Copy link
Member

@Asmod4n Asmod4n commented Feb 23, 2014

Added two new Configuration options, :runs_behind_proxy and :trusted_headers, when :runs_behind_proxy is set to true each new Request filters the headers Hash and modifies the request.uri in Webmachine::Request

Webmachine::Headers[‘foo’] always returns something, even when its not
what was requested oO
@ghost
Copy link

ghost commented Feb 24, 2014

replying to a5550a2#commitcomment-5464134
it seems like it would be a good idea to delegate Webmachine::Application through Request and maybe other objects that need it.

@Asmod4n
Copy link
Member Author

Asmod4n commented Feb 24, 2014

shrug would still involve class vars or rewriting of each adapter.

@ghost
Copy link

ghost commented Feb 24, 2014

it doesn't have to involve class variables.

class Webmachine::Application # last time i write this tonight
  class << self
    attr_accessor :proxy_support
  end
end

that example uses an instance variable. it can be localized in an instance of Application, and then you could avoid:

class Webmachine::Request
  def initialize(*)
    if Application.proxy_support
      
    end
  end
end

@ghost
Copy link

ghost commented Feb 24, 2014

it's a possible simple solution to the problem because if one application changes global state then the other won't see it, as is probably intended. that means the global state becomes a "default" but that the application owns it once you create an instance of it.

@Asmod4n
Copy link
Member Author

Asmod4n commented Feb 24, 2014

Meaning you have to change https://github.com/seancribbs/webmachine-ruby/blob/master/lib/webmachine/request.rb#L33 to

def initialize(method, uri, headers, body, configuration)

and have each adapter do this:

Webmachine::Request.new(method, uri, wm_headers, request.body, configuration)

because thats the only place you can get a hold of that variable.

doing @configuration = Adapter.configuration.dup inside Webmachine::Request means you always get the newest configuration from another instance of Webmachine::Application.

@ghost
Copy link

ghost commented Feb 24, 2014

yeah, you have to change Request to accept an instance of Application. it's a backwards incompatible change. if you're uncomfortable with that maybe @seancribbs has some ideas.

it feels okay to me because the instance of Request is always local to an instance of Application, so forwarding on what is already local to keep stuff local seems okay.

@ghost
Copy link

ghost commented Feb 24, 2014

btw i would probably just pass an instance of Application, it seems more useful than Configuration and it can be responsible for not holding onto global state from stuff like Application.proxy_support. that's just my general feeling.

@Asmod4n
Copy link
Member Author

Asmod4n commented Feb 24, 2014

Have changed webrick that way, will go to sleep now ^^

@ghost
Copy link

ghost commented Feb 24, 2014

sleep well!

@Asmod4n Asmod4n closed this Feb 24, 2014
@Asmod4n Asmod4n deleted the proxy_support branch February 24, 2014 12:16
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

Successfully merging this pull request may close these issues.

1 participant