-
Notifications
You must be signed in to change notification settings - Fork 326
add support for users to provide custom uri normalizer #533
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
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| module HTTP | ||
| module Features | ||
| class UriNormalizer < Feature | ||
| attr_reader :custom_uri_normalizer | ||
|
|
||
| def initialize(custom_uri_normalizer: DefaultUriNormalizer.new) | ||
| @custom_uri_normalizer = custom_uri_normalizer | ||
| end | ||
|
|
||
| def normalize_uri(uri) | ||
| custom_uri_normalizer.normalize_uri(uri) | ||
| end | ||
|
|
||
| class DefaultUriNormalizer | ||
| def normalize_uri(uri) | ||
| uri = HTTP::URI.parse uri | ||
| HTTP::URI.new( | ||
| :scheme => uri.normalized_scheme, | ||
| :authority => uri.normalized_authority, | ||
| :path => uri.normalized_path, | ||
| :query => uri.query, | ||
| :fragment => uri.normalized_fragment | ||
| ) | ||
| end | ||
| end | ||
|
|
||
| HTTP::Options.register_feature(:uri_normalizer, self) | ||
| end | ||
| end | ||
| end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -66,20 +66,24 @@ class UnsupportedSchemeError < RequestError; end | |
| # Scheme is normalized to be a lowercase symbol e.g. :http, :https | ||
| attr_reader :scheme | ||
|
|
||
| attr_reader :uri_normalizer | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No need to expose it.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am exposing it because we are using it in |
||
|
|
||
| # "Request URI" as per RFC 2616 | ||
| # http://www.w3.org/Protocols/rfc2616/rfc2616-sec5.html | ||
| attr_reader :uri | ||
| attr_reader :proxy, :body, :version | ||
|
|
||
| # @option opts [String] :version | ||
| # @option opts [#to_s] :verb HTTP request method | ||
| # @option opts [#to_s] :uri_normalizer uri normalizer | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. description is redundant. It's obvious from the key name. |
||
| # @option opts [HTTP::URI, #to_s] :uri | ||
| # @option opts [Hash] :headers | ||
| # @option opts [Hash] :proxy | ||
| # @option opts [String, Enumerable, IO, nil] :body | ||
| def initialize(opts) | ||
| @verb = opts.fetch(:verb).to_s.downcase.to_sym | ||
| @uri = normalize_uri(opts.fetch(:uri)) | ||
| @uri_normalizer = opts[:uri_normalizer] ? opts.fetch(:uri_normalizer) : self.class.method(:normalize_uri) | ||
| @uri = @uri_normalizer.call(opts.fetch(:uri)) | ||
|
mamoonraja marked this conversation as resolved.
Outdated
|
||
| @scheme = @uri.scheme.to_s.downcase.to_sym if @uri.scheme | ||
|
|
||
| raise(UnsupportedMethodError, "unknown method: #{verb}") unless METHODS.include?(@verb) | ||
|
|
@@ -196,6 +200,19 @@ def inspect | |
| "#<#{self.class}/#{@version} #{verb.to_s.upcase} #{uri}>" | ||
| end | ||
|
|
||
| # @return [HTTP::URI] URI with all components but query being normalized. | ||
| def self.normalize_uri(uri) | ||
|
mamoonraja marked this conversation as resolved.
Outdated
|
||
| uri = HTTP::URI.parse uri | ||
|
|
||
| HTTP::URI.new( | ||
| :scheme => uri.normalized_scheme, | ||
| :authority => uri.normalized_authority, | ||
| :path => uri.normalized_path, | ||
| :query => uri.query, | ||
| :fragment => uri.normalized_fragment | ||
| ) | ||
| end | ||
|
|
||
| private | ||
|
|
||
| # @!attribute [r] host | ||
|
|
@@ -212,18 +229,5 @@ def port | |
| def default_host_header_value | ||
| PORTS[@scheme] != port ? "#{host}:#{port}" : host | ||
| end | ||
|
|
||
| # @return [HTTP::URI] URI with all componentes but query being normalized. | ||
| def normalize_uri(uri) | ||
| uri = HTTP::URI.parse uri | ||
|
|
||
| HTTP::URI.new( | ||
| :scheme => uri.normalized_scheme, | ||
| :authority => uri.normalized_authority, | ||
| :path => uri.normalized_path, | ||
| :query => uri.query, | ||
| :fragment => uri.normalized_fragment | ||
| ) | ||
| end | ||
| end | ||
| end | ||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -430,6 +430,38 @@ def setsockopt(*args) | |||||
| expect(response.to_s).to eq("#{body}-deflated") | ||||||
| end | ||||||
| end | ||||||
|
|
||||||
| context "with :uri_normalizer" do | ||||||
| class CustomUriNormalizer | ||||||
| def normalize_uri(uri) | ||||||
| uri = HTTP::URI.parse uri | ||||||
| HTTP::URI.new( | ||||||
| :scheme => uri.normalized_scheme, | ||||||
| :authority => uri.normalized_authority, | ||||||
| :path => "uri_normalizer/custom", | ||||||
| :query => uri.query, | ||||||
| :fragment => uri.normalized_fragment | ||||||
| ) | ||||||
| end | ||||||
| end | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't declare module/class dynamically. You basically don't need this at all. |
||||||
|
|
||||||
| it "Use the defaul Uri normalizer when user does not use uri normalizer" do | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. URI should be all caps. And topic of example should start with lowercase. Also typo in defaul. Line 21 in 1eae155
or here: Line 466 in 1eae155
And should look like: it "normalizes URI" do
response = HTTP.get "#{dummy.endpoint}/hello world"
expect(response.to_s).to eq("hello world")
endothers examples are OK to be left in here, just make them less "wordy"... |
||||||
| response = HTTP.get HTTP::URI.parse "#{dummy.endpoint}/uri_normalizer/%EF%BC%A1%EF%BC%A2%EF%BC%A3" | ||||||
| expect(response.to_s).to eq("default normalizer") | ||||||
| end | ||||||
|
|
||||||
| it "Use the custom Uri Normalizer method" do | ||||||
| client = HTTP.use(:uri_normalizer => {:custom_uri_normalizer => CustomUriNormalizer.new}) | ||||||
| response = client.get("#{dummy.endpoint}/uri_normalizer/%EF%BC%A1%EF%BC%A2%EF%BC%A3") | ||||||
| expect(response.to_s).to eq("custom normalizer") | ||||||
| end | ||||||
|
|
||||||
| it "Use the default Uri normalizer when user does not specify custom uri normalizer" do | ||||||
| client = HTTP.use :uri_normalizer | ||||||
| response = client.get("#{dummy.endpoint}/uri_normalizer/%EF%BC%A1%EF%BC%A2%EF%BC%A3") | ||||||
| expect(response.to_s).to eq("default normalizer") | ||||||
| end | ||||||
| end | ||||||
| end | ||||||
|
|
||||||
| it "unifies socket errors into HTTP::ConnectionError" do | ||||||
|
|
||||||
Uh oh!
There was an error while loading. Please reload this page.