From a67d5c340431e66d87dab87cb2cc36488d86e862 Mon Sep 17 00:00:00 2001 From: Ryan Loomba Date: Wed, 17 Jun 2020 01:14:23 -0700 Subject: [PATCH 1/5] add ability to use http proxy when making web requests --- README.md | 3 +- lib/optimizely/config/proxy_config.rb | 16 ++++++ .../http_project_config_manager.rb | 7 ++- lib/optimizely/event_dispatcher.rb | 5 +- lib/optimizely/helpers/http_utils.rb | 23 ++++++-- spec/config/proxy_config_spec.rb | 44 +++++++++++++++ .../http_project_config_manager_spec.rb | 16 +++++- spec/event_dispatcher_spec.rb | 21 ++++++- spec/helpers/http_utils_spec.rb | 55 +++++++++++++++++++ 9 files changed, 177 insertions(+), 13 deletions(-) create mode 100644 lib/optimizely/config/proxy_config.rb create mode 100644 spec/config/proxy_config_spec.rb create mode 100644 spec/helpers/http_utils_spec.rb diff --git a/README.md b/README.md index 7965ea50..36ecd396 100644 --- a/README.md +++ b/README.md @@ -88,7 +88,8 @@ The `HTTPConfigManager` asynchronously polls for datafiles from a specified URL error_handler: nil, skip_json_validation: false, notification_center: notification_center, - datafile_access_token: nil + datafile_access_token: nil, + proxy_config: nil ) ~~~~~~ **Note:** You must provide either the `sdk_key` or URL. If you provide both, the URL takes precedence. diff --git a/lib/optimizely/config/proxy_config.rb b/lib/optimizely/config/proxy_config.rb new file mode 100644 index 00000000..12a9913e --- /dev/null +++ b/lib/optimizely/config/proxy_config.rb @@ -0,0 +1,16 @@ +module Optimizely + class ProxyConfig + attr_reader :host, :port, :username, :password + + def initialize(host, port = nil, username = nil, password = nil) + # host - DNS name or IP address of proxy + # port - port to use to acess the proxy + # username - username if authorization is required + # password - password if authorization is required + @host = host + @port = port + @username = username + @password = password + end + end +end diff --git a/lib/optimizely/config_manager/http_project_config_manager.rb b/lib/optimizely/config_manager/http_project_config_manager.rb index 411f5744..06a4952e 100644 --- a/lib/optimizely/config_manager/http_project_config_manager.rb +++ b/lib/optimizely/config_manager/http_project_config_manager.rb @@ -52,6 +52,7 @@ class HTTPProjectConfigManager < ProjectConfigManager # skip_json_validation - Optional boolean param which allows skipping JSON schema # validation upon object invocation. By default JSON schema validation will be performed. # datafile_access_token - access token used to fetch private datafiles + # proxy_config - Optional proxy config instancea to configure making web requests through a proxy server. def initialize( sdk_key: nil, url: nil, @@ -65,7 +66,8 @@ def initialize( error_handler: nil, skip_json_validation: false, notification_center: nil, - datafile_access_token: nil + datafile_access_token: nil, + proxy_config: nil ) @logger = logger || NoOpLogger.new @error_handler = error_handler || NoOpErrorHandler.new @@ -86,6 +88,7 @@ def initialize( # Start async scheduler in the end to avoid race condition where scheduler executes # callback which makes use of variables not yet initialized by the main thread. @async_scheduler.start! if start_by_default == true + @proxy_config = proxy_config @stopped = false end @@ -159,7 +162,7 @@ def request_config headers['Authorization'] = "Bearer #{@access_token}" unless @access_token.nil? response = Helpers::HttpUtils.make_request( - @datafile_url, :get, nil, headers, Helpers::Constants::CONFIG_MANAGER['REQUEST_TIMEOUT'] + @datafile_url, :get, nil, headers, Helpers::Constants::CONFIG_MANAGER['REQUEST_TIMEOUT'], @proxy_config ) rescue StandardError => e @logger.log( diff --git a/lib/optimizely/event_dispatcher.rb b/lib/optimizely/event_dispatcher.rb index 7f480b16..18e4e57c 100644 --- a/lib/optimizely/event_dispatcher.rb +++ b/lib/optimizely/event_dispatcher.rb @@ -29,9 +29,10 @@ class EventDispatcher # @api constants REQUEST_TIMEOUT = 10 - def initialize(logger: nil, error_handler: nil) + def initialize(logger: nil, error_handler: nil, proxy_config: nil) @logger = logger || NoOpLogger.new @error_handler = error_handler || NoOpErrorHandler.new + @proxy_config = proxy_config end # Dispatch the event being represented by the Event object. @@ -39,7 +40,7 @@ def initialize(logger: nil, error_handler: nil) # @param event - Event object def dispatch_event(event) response = Helpers::HttpUtils.make_request( - event.url, event.http_verb, event.params.to_json, event.headers, REQUEST_TIMEOUT + event.url, event.http_verb, event.params.to_json, event.headers, REQUEST_TIMEOUT, @proxy_config ) error_msg = "Event failed to dispatch with response code: #{response.code}" diff --git a/lib/optimizely/helpers/http_utils.rb b/lib/optimizely/helpers/http_utils.rb index bea26a7a..c590f782 100644 --- a/lib/optimizely/helpers/http_utils.rb +++ b/lib/optimizely/helpers/http_utils.rb @@ -23,14 +23,10 @@ module Helpers module HttpUtils module_function - def make_request(url, http_method, request_body = nil, headers = {}, read_timeout = nil) + def make_request(url, http_method, request_body = nil, headers = {}, read_timeout = nil, proxy_config = nil) # makes http/https GET/POST request and returns response - + # uri = URI.parse(url) - http = Net::HTTP.new(uri.host, uri.port) - - http.read_timeout = read_timeout if read_timeout - http.use_ssl = uri.scheme == 'https' if http_method == :get request = Net::HTTP::Get.new(uri.request_uri) @@ -46,6 +42,21 @@ def make_request(url, http_method, request_body = nil, headers = {}, read_timeou request[key] = val end + # do not try to make request with proxy unless we have at least a host + http_class = if proxy_config && proxy_config.host + Net::HTTP::Proxy( + proxy_config.host, + proxy_config.port, + proxy_config.username, + proxy_config.password + ) + else + Net::HTTP + end + + http = http_class.new(uri.host, uri.port) + http.read_timeout = read_timeout if read_timeout + http.use_ssl = uri.scheme == 'https' http.request(request) end end diff --git a/spec/config/proxy_config_spec.rb b/spec/config/proxy_config_spec.rb new file mode 100644 index 00000000..4255acf3 --- /dev/null +++ b/spec/config/proxy_config_spec.rb @@ -0,0 +1,44 @@ +# frozen_string_literal: true + +# +# Copyright 2019-2020, Optimizely and contributors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +require 'spec_helper' +require 'optimizely/config/proxy_config' + +describe Optimizely::ProxyConfig do + let(:host) { 'host' } + let(:port) { 1234 } + let(:username) { 'username' } + let(:password) { 'password' } + + describe '#initialize' do + it 'defines getters for host, port, username, and password' do + proxy_config = described_class.new(host, port, username, password) + + expect(proxy_config.host).to eq(host) + expect(proxy_config.port).to eq(port) + expect(proxy_config.username).to eq(username) + expect(proxy_config.password).to eq(password) + end + + it 'sets port, username, and password to nil if they are not passed in' do + proxy_config = described_class.new(host) + expect(proxy_config.port).to eq(nil) + expect(proxy_config.username).to eq(nil) + expect(proxy_config.password).to eq(nil) + end + end +end diff --git a/spec/config_manager/http_project_config_manager_spec.rb b/spec/config_manager/http_project_config_manager_spec.rb index 5576a47b..172610a8 100644 --- a/spec/config_manager/http_project_config_manager_spec.rb +++ b/spec/config_manager/http_project_config_manager_spec.rb @@ -472,7 +472,7 @@ datafile_access_token: 'the-token' ) sleep 0.1 - expect(Optimizely::Helpers::HttpUtils).to have_received(:make_request).with(anything, anything, anything, hash_including('Authorization' => 'Bearer the-token'), anything) + expect(Optimizely::Helpers::HttpUtils).to have_received(:make_request).with(anything, anything, anything, hash_including('Authorization' => 'Bearer the-token'), anything, anything) end it 'should use authenticated datafile url when auth token is provided' do @@ -504,5 +504,19 @@ sleep 0.1 expect(Optimizely::Helpers::HttpUtils).to have_received(:make_request).with('http://awesomeurl', any_args) end + + it 'should pass the proxy config that is passed in' do + proxy_config = double(:proxy_config) + + allow(Optimizely::Helpers::HttpUtils).to receive(:make_request) + @http_project_config_manager = Optimizely::HTTPProjectConfigManager.new( + sdk_key: 'valid_sdk_key', + datafile_access_token: 'the-token', + proxy_config: proxy_config + ) + sleep 0.1 + expect(Optimizely::Helpers::HttpUtils).to have_received(:make_request).with(anything, anything, anything, hash_including('Authorization' => 'Bearer the-token'), anything, proxy_config) + + end end end diff --git a/spec/event_dispatcher_spec.rb b/spec/event_dispatcher_spec.rb index 420d5345..151f31ab 100644 --- a/spec/event_dispatcher_spec.rb +++ b/spec/event_dispatcher_spec.rb @@ -22,6 +22,7 @@ describe Optimizely::EventDispatcher do let(:error_handler) { spy(Optimizely::NoOpErrorHandler.new) } let(:spy_logger) { spy('logger') } + let(:proxy_config) { nil } before(:context) do @url = 'https://www.optimizely.com' @@ -37,10 +38,28 @@ before(:example) do @event_dispatcher = Optimizely::EventDispatcher.new @customized_event_dispatcher = Optimizely::EventDispatcher.new( - logger: spy_logger, error_handler: error_handler + logger: spy_logger, error_handler: error_handler, proxy_config: proxy_config ) end + context 'passing in proxy config' do + let(:proxy_config) { double(:proxy_config) } + + it 'should pass the proxy_config to the HttpUtils helper class' do + event = Optimizely::Event.new(:post, @url, @params, @post_headers) + expect(Optimizely::Helpers::HttpUtils).to receive(:make_request).with( + event.url, + event.http_verb, + event.params.to_json, + event.headers, + Optimizely::EventDispatcher::REQUEST_TIMEOUT, + proxy_config + ) + + @customized_event_dispatcher.dispatch_event(event) + end + end + it 'should properly dispatch V2 (POST) events' do stub_request(:post, @url) event = Optimizely::Event.new(:post, @url, @params, @post_headers) diff --git a/spec/helpers/http_utils_spec.rb b/spec/helpers/http_utils_spec.rb new file mode 100644 index 00000000..8d3ba50e --- /dev/null +++ b/spec/helpers/http_utils_spec.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +# Copyright 2016-2017, 2019-2020, Optimizely and contributors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +# +require 'spec_helper' +#require 'optimizely/helpers/http_utils' +require 'optimizely/config/proxy_config' + +describe Optimizely::Helpers::HttpUtils do + context 'passing in a proxy config' do + let(:url) { 'https://example.com' } + let(:http_method) { :get } + let(:host) { 'host' } + let(:port) { 1234 } + let(:username) { 'username' } + let(:password) { 'password' } + let(:http_class) { double(:http_class) } + let(:http) { double(:http) } + + before do + allow(http_class).to receive(:new).and_return(http) + allow(http).to receive(:use_ssl=) + allow(http).to receive(:request) + end + + context 'with a proxy config that inclues host, port, username, and password' do + let(:proxy_config) { Optimizely::ProxyConfig.new(host, port, username, password) } + it 'with a full proxy config, it proxies the web request' do + expect(Net::HTTP).to receive(:Proxy).with(host, port, username, password).and_return(http_class) + described_class.make_request(url, http_method, nil, nil, nil, proxy_config) + end + end + + context 'with a proxy config that only inclues host' do + let(:proxy_config) { Optimizely::ProxyConfig.new(host) } + it 'with a full proxy config, it proxies the web request' do + expect(Net::HTTP).to receive(:Proxy).with(host, nil, nil, nil).and_return(http_class) + described_class.make_request(url, http_method, nil, nil, nil, proxy_config) + end + end + end +end From a0475e138b0b02308248d1cf759f5c302d672fce Mon Sep 17 00:00:00 2001 From: Owais Akbani Date: Wed, 8 Jul 2020 19:40:05 +0500 Subject: [PATCH 2/5] fix: linting --- .rubocop_todo.yml | 2 +- lib/optimizely/config/proxy_config.rb | 4 +++- lib/optimizely/helpers/http_utils.rb | 20 +++++++++---------- .../http_project_config_manager_spec.rb | 2 -- spec/helpers/http_utils_spec.rb | 2 +- 5 files changed, 15 insertions(+), 15 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index c49d5397..6da46802 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -14,7 +14,7 @@ Lint/HandleExceptions: # Offense count: 8 # Configuration parameters: CountKeywordArgs. Metrics/ParameterLists: - Max: 13 + Max: 14 # Offense count: 2 Naming/AccessorMethodName: diff --git a/lib/optimizely/config/proxy_config.rb b/lib/optimizely/config/proxy_config.rb index 12a9913e..1b97a616 100644 --- a/lib/optimizely/config/proxy_config.rb +++ b/lib/optimizely/config/proxy_config.rb @@ -1,8 +1,10 @@ +# frozen_string_literal: true + module Optimizely class ProxyConfig attr_reader :host, :port, :username, :password - def initialize(host, port = nil, username = nil, password = nil) + def initialize(host, port = nil, username = nil, password = nil) # host - DNS name or IP address of proxy # port - port to use to acess the proxy # username - username if authorization is required diff --git a/lib/optimizely/helpers/http_utils.rb b/lib/optimizely/helpers/http_utils.rb index c590f782..fae0aa20 100644 --- a/lib/optimizely/helpers/http_utils.rb +++ b/lib/optimizely/helpers/http_utils.rb @@ -43,16 +43,16 @@ def make_request(url, http_method, request_body = nil, headers = {}, read_timeou end # do not try to make request with proxy unless we have at least a host - http_class = if proxy_config && proxy_config.host - Net::HTTP::Proxy( - proxy_config.host, - proxy_config.port, - proxy_config.username, - proxy_config.password - ) - else - Net::HTTP - end + http_class = if proxy_config&.host + Net::HTTP::Proxy( + proxy_config.host, + proxy_config.port, + proxy_config.username, + proxy_config.password + ) + else + Net::HTTP + end http = http_class.new(uri.host, uri.port) http.read_timeout = read_timeout if read_timeout diff --git a/spec/config_manager/http_project_config_manager_spec.rb b/spec/config_manager/http_project_config_manager_spec.rb index 2a11be5b..1a79bb69 100644 --- a/spec/config_manager/http_project_config_manager_spec.rb +++ b/spec/config_manager/http_project_config_manager_spec.rb @@ -527,7 +527,6 @@ ) sleep 0.1 expect(Optimizely::Helpers::HttpUtils).to have_received(:make_request).with(anything, anything, anything, hash_including('Authorization' => 'Bearer the-token'), anything, proxy_config) - end it 'should pass the proxy config that is passed in' do @@ -541,7 +540,6 @@ ) sleep 0.1 expect(Optimizely::Helpers::HttpUtils).to have_received(:make_request).with(anything, anything, anything, hash_including('Authorization' => 'Bearer the-token'), anything, proxy_config) - end end end diff --git a/spec/helpers/http_utils_spec.rb b/spec/helpers/http_utils_spec.rb index 8d3ba50e..2b6d4f47 100644 --- a/spec/helpers/http_utils_spec.rb +++ b/spec/helpers/http_utils_spec.rb @@ -16,7 +16,7 @@ # # require 'spec_helper' -#require 'optimizely/helpers/http_utils' +# require 'optimizely/helpers/http_utils' require 'optimizely/config/proxy_config' describe Optimizely::Helpers::HttpUtils do From 44be9f208b610214aeeb77c3bf1a0ba0e9bdfb50 Mon Sep 17 00:00:00 2001 From: Owais Akbani Date: Wed, 8 Jul 2020 19:47:44 +0500 Subject: [PATCH 3/5] ignore spellcheck --- .travis.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 19a1045b..6626ae7b 100644 --- a/.travis.yml +++ b/.travis.yml @@ -43,7 +43,8 @@ jobs: install: - npm i -g markdown-spellcheck before_script: - - wget --quiet https://raw.githubusercontent.com/optimizely/mdspell-config/master/.spelling + # todo: change branch to master once merged. + - wget --quiet https://raw.githubusercontent.com/optimizely/mdspell-config/oakbani/ignore-for-ruby/.spelling script: - mdspell -a -n -r --en-us '**/*.md' after_success: skip From 0873952fada98eca0c79d57c8b05ad5130ca58d7 Mon Sep 17 00:00:00 2001 From: Owais Akbani Date: Wed, 8 Jul 2020 19:58:38 +0500 Subject: [PATCH 4/5] headers and missing test --- lib/optimizely/config/proxy_config.rb | 16 ++++++++++++++++ spec/config/proxy_config_spec.rb | 2 +- .../http_project_config_manager_spec.rb | 8 +++----- spec/helpers/http_utils_spec.rb | 3 +-- 4 files changed, 21 insertions(+), 8 deletions(-) diff --git a/lib/optimizely/config/proxy_config.rb b/lib/optimizely/config/proxy_config.rb index 1b97a616..6385ac7b 100644 --- a/lib/optimizely/config/proxy_config.rb +++ b/lib/optimizely/config/proxy_config.rb @@ -1,5 +1,21 @@ # frozen_string_literal: true +# Copyright 2020, Optimizely and contributors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +# + module Optimizely class ProxyConfig attr_reader :host, :port, :username, :password diff --git a/spec/config/proxy_config_spec.rb b/spec/config/proxy_config_spec.rb index 4255acf3..fde658a9 100644 --- a/spec/config/proxy_config_spec.rb +++ b/spec/config/proxy_config_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true # -# Copyright 2019-2020, Optimizely and contributors +# Copyright 2020, Optimizely and contributors # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. diff --git a/spec/config_manager/http_project_config_manager_spec.rb b/spec/config_manager/http_project_config_manager_spec.rb index 1a79bb69..038efe14 100644 --- a/spec/config_manager/http_project_config_manager_spec.rb +++ b/spec/config_manager/http_project_config_manager_spec.rb @@ -516,17 +516,15 @@ expect(Optimizely::Helpers::HttpUtils).to have_received(:make_request).with('http://awesomeurl', any_args) end - it 'should pass the proxy config that is passed in' do - proxy_config = double(:proxy_config) - + it 'should hide access token when printing logs' do allow(Optimizely::Helpers::HttpUtils).to receive(:make_request) @http_project_config_manager = Optimizely::HTTPProjectConfigManager.new( sdk_key: 'valid_sdk_key', datafile_access_token: 'the-token', - proxy_config: proxy_config + logger: spy_logger ) sleep 0.1 - expect(Optimizely::Helpers::HttpUtils).to have_received(:make_request).with(anything, anything, anything, hash_including('Authorization' => 'Bearer the-token'), anything, proxy_config) + expect(spy_logger).to have_received(:log).with(Logger::DEBUG, 'Datafile request headers: {"Content-Type"=>"application/json", "Authorization"=>"********"}').once end it 'should pass the proxy config that is passed in' do diff --git a/spec/helpers/http_utils_spec.rb b/spec/helpers/http_utils_spec.rb index 2b6d4f47..75b043c5 100644 --- a/spec/helpers/http_utils_spec.rb +++ b/spec/helpers/http_utils_spec.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -# Copyright 2016-2017, 2019-2020, Optimizely and contributors +# Copyright 2020, Optimizely and contributors # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -16,7 +16,6 @@ # # require 'spec_helper' -# require 'optimizely/helpers/http_utils' require 'optimizely/config/proxy_config' describe Optimizely::Helpers::HttpUtils do From 0adc733ae7e5dab2302ade692b8e0b22553eebef Mon Sep 17 00:00:00 2001 From: Tom Zurkan Date: Wed, 8 Jul 2020 15:37:23 -0400 Subject: [PATCH 5/5] update to use master mdspell --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 6626ae7b..672f16a3 100644 --- a/.travis.yml +++ b/.travis.yml @@ -44,7 +44,7 @@ jobs: - npm i -g markdown-spellcheck before_script: # todo: change branch to master once merged. - - wget --quiet https://raw.githubusercontent.com/optimizely/mdspell-config/oakbani/ignore-for-ruby/.spelling + - wget --quiet https://raw.githubusercontent.com/optimizely/mdspell-config/master/.spelling script: - mdspell -a -n -r --en-us '**/*.md' after_success: skip