Commit a1c29241894269cda717e7c5893ec9b6d614097d
1 parent
218cf0a5
Exists in
send_email_to_admins
and in
5 other branches
isolate proxy settings among multiple environments
Showing
3 changed files
with
61 additions
and
39 deletions
Show diff stats
app/models/external_feed.rb
| ... | ... | @@ -12,6 +12,8 @@ class ExternalFeed < ActiveRecord::Base |
| 12 | 12 | |
| 13 | 13 | attr_accessible :address, :enabled, :only_once |
| 14 | 14 | |
| 15 | + delegate :environment, :to => :blog, :allow_nil => true | |
| 16 | + | |
| 15 | 17 | def add_item(title, link, date, content) |
| 16 | 18 | return if content.blank? |
| 17 | 19 | doc = Nokogiri::HTML.fragment content | ... | ... |
lib/feed_handler.rb
| ... | ... | @@ -31,7 +31,7 @@ class FeedHandler |
| 31 | 31 | end |
| 32 | 32 | end |
| 33 | 33 | |
| 34 | - def fetch(address) | |
| 34 | + def fetch(address, header = {}) | |
| 35 | 35 | begin |
| 36 | 36 | content = "" |
| 37 | 37 | block = lambda { |s| content = s.read } |
| ... | ... | @@ -42,21 +42,7 @@ class FeedHandler |
| 42 | 42 | if !valid_url?(address) |
| 43 | 43 | raise InvalidUrl.new("\"%s\" is not a valid URL" % address) |
| 44 | 44 | end |
| 45 | - | |
| 46 | - header = {"User-Agent" => "Noosfero/#{Noosfero::VERSION}"} | |
| 47 | - | |
| 48 | - environment = Environment.default | |
| 49 | - | |
| 50 | - if environment.enable_feed_proxy | |
| 51 | - if address.starts_with?("https://") | |
| 52 | - header.merge!(:proxy => environment.https_feed_proxy) if environment.https_feed_proxy | |
| 53 | - else | |
| 54 | - header.merge!(:proxy => environment.http_feed_proxy) if environment.http_feed_proxy | |
| 55 | - end | |
| 56 | - end | |
| 57 | - | |
| 58 | - header.merge!(:ssl_verify_mode => OpenSSL::SSL::VERIFY_NONE) if environment.disable_feed_ssl | |
| 59 | - | |
| 45 | + header.merge!("User-Agent" => "Noosfero/#{Noosfero::VERSION}") | |
| 60 | 46 | open(address, header, &block) |
| 61 | 47 | end |
| 62 | 48 | return content |
| ... | ... | @@ -65,6 +51,17 @@ class FeedHandler |
| 65 | 51 | end |
| 66 | 52 | end |
| 67 | 53 | |
| 54 | + def fetch_through_proxy(address, environment) | |
| 55 | + header = {} | |
| 56 | + if address.starts_with?("https://") | |
| 57 | + header.merge!(:proxy => environment.https_feed_proxy) if environment.https_feed_proxy | |
| 58 | + else | |
| 59 | + header.merge!(:proxy => environment.http_feed_proxy) if environment.http_feed_proxy | |
| 60 | + end | |
| 61 | + header.merge!(:ssl_verify_mode => OpenSSL::SSL::VERIFY_NONE) if environment.disable_feed_ssl | |
| 62 | + fetch(address, header) | |
| 63 | + end | |
| 64 | + | |
| 68 | 65 | def process(container) |
| 69 | 66 | begin |
| 70 | 67 | container.class.transaction do |
| ... | ... | @@ -105,7 +102,11 @@ class FeedHandler |
| 105 | 102 | |
| 106 | 103 | def actually_process_container(container) |
| 107 | 104 | container.clear |
| 108 | - content = fetch(container.address) | |
| 105 | + if container.environment.enable_feed_proxy | |
| 106 | + content = fetch_through_proxy(container.address, container.environment) | |
| 107 | + else | |
| 108 | + content = fetch(container.address) | |
| 109 | + end | |
| 109 | 110 | container.fetched_at = Time.now |
| 110 | 111 | parsed_feed = parse(content) |
| 111 | 112 | container.feed_title = parsed_feed.title | ... | ... |
test/unit/feed_handler_test.rb
| ... | ... | @@ -27,6 +27,7 @@ class FeedHandlerTest < ActiveSupport::TestCase |
| 27 | 27 | end |
| 28 | 28 | |
| 29 | 29 | should 'process feed and populate container' do |
| 30 | + container.stubs(:environment).returns(Environment.new) | |
| 30 | 31 | handler.process(container) |
| 31 | 32 | assert_equal 'Feed for unit tests', container.feed_title |
| 32 | 33 | assert_equivalent ["First POST", "Second POST", "Last POST"], container.feed_items.map {|item| item[:title]} |
| ... | ... | @@ -61,6 +62,7 @@ class FeedHandlerTest < ActiveSupport::TestCase |
| 61 | 62 | end |
| 62 | 63 | |
| 63 | 64 | should 'save only latest N posts from feed' do |
| 65 | + container.stubs(:environment).returns(Environment.new) | |
| 64 | 66 | container.limit = 1 |
| 65 | 67 | handler.process(container) |
| 66 | 68 | assert_equal 1, container.feed_items.size |
| ... | ... | @@ -83,9 +85,6 @@ class FeedHandlerTest < ActiveSupport::TestCase |
| 83 | 85 | end |
| 84 | 86 | |
| 85 | 87 | should 'identifies itself as noosfero user agent' do |
| 86 | - Environment.any_instance.expects(:enable_feed_proxy).returns(false) | |
| 87 | - Environment.any_instance.expects(:disable_feed_ssl).returns(false) | |
| 88 | - | |
| 89 | 88 | handler.expects(:open).with('http://site.org/feed.xml', {"User-Agent" => "Noosfero/#{Noosfero::VERSION}"}, anything).returns('bli content') |
| 90 | 89 | |
| 91 | 90 | assert_equal 'bli content', handler.fetch('http://site.org/feed.xml') |
| ... | ... | @@ -154,52 +153,72 @@ class FeedHandlerTest < ActiveSupport::TestCase |
| 154 | 153 | end |
| 155 | 154 | |
| 156 | 155 | should 'set proxy when http_feed_proxy is setted from env' do |
| 157 | - Environment.any_instance.expects(:enable_feed_proxy).returns(true) | |
| 158 | - Environment.any_instance.expects(:disable_feed_ssl).returns(false) | |
| 159 | - Environment.any_instance.expects(:http_feed_proxy).twice.returns('http://127.0.0.1:3128') | |
| 156 | + env = Environment.new | |
| 157 | + env.expects(:disable_feed_ssl).returns(false) | |
| 158 | + env.expects(:http_feed_proxy).twice.returns('http://127.0.0.1:3128') | |
| 160 | 159 | |
| 161 | 160 | handler.expects(:open).with('http://site.org/feed.xml', {"User-Agent" => "Noosfero/#{Noosfero::VERSION}", :proxy => 'http://127.0.0.1:3128'}, anything).returns('bli content') |
| 162 | 161 | |
| 163 | - assert_equal 'bli content', handler.fetch('http://site.org/feed.xml') | |
| 162 | + assert_equal 'bli content', handler.fetch_through_proxy('http://site.org/feed.xml', env) | |
| 164 | 163 | end |
| 165 | 164 | |
| 166 | 165 | should 'set proxy when https_feed_proxy is setted from env' do |
| 167 | - Environment.any_instance.expects(:enable_feed_proxy).returns(true) | |
| 168 | - Environment.any_instance.expects(:disable_feed_ssl).returns(false) | |
| 169 | - Environment.any_instance.expects(:https_feed_proxy).twice.returns('http://127.0.0.1:3128') | |
| 166 | + env = Environment.new | |
| 167 | + env.expects(:disable_feed_ssl).returns(false) | |
| 168 | + env.expects(:https_feed_proxy).twice.returns('http://127.0.0.1:3128') | |
| 170 | 169 | |
| 171 | 170 | handler.expects(:open).with('https://site.org/feed.xml', {"User-Agent" => "Noosfero/#{Noosfero::VERSION}", :proxy => 'http://127.0.0.1:3128'}, anything).returns('bli content') |
| 172 | 171 | |
| 173 | - assert_equal 'bli content', handler.fetch('https://site.org/feed.xml') | |
| 172 | + assert_equal 'bli content', handler.fetch_through_proxy('https://site.org/feed.xml', env) | |
| 174 | 173 | end |
| 175 | 174 | |
| 176 | 175 | should 'use https proxy for https address when both env variables were defined' do |
| 177 | - Environment.any_instance.expects(:enable_feed_proxy).returns(true) | |
| 178 | - Environment.any_instance.expects(:disable_feed_ssl).returns(false) | |
| 179 | - Environment.any_instance.expects(:https_feed_proxy).twice.returns('http://127.0.0.2:3128') | |
| 176 | + env = Environment.new | |
| 177 | + env.expects(:disable_feed_ssl).returns(false) | |
| 178 | + env.expects(:https_feed_proxy).twice.returns('http://127.0.0.2:3128') | |
| 180 | 179 | |
| 181 | 180 | handler.expects(:open).with('https://site.org/feed.xml', {"User-Agent" => "Noosfero/#{Noosfero::VERSION}", :proxy => 'http://127.0.0.2:3128'}, anything).returns('bli content') |
| 182 | 181 | |
| 183 | - assert_equal 'bli content', handler.fetch('https://site.org/feed.xml') | |
| 182 | + assert_equal 'bli content', handler.fetch_through_proxy('https://site.org/feed.xml', env) | |
| 184 | 183 | end |
| 185 | 184 | |
| 186 | 185 | should 'use http proxy for http address when both env variables were defined' do |
| 187 | - Environment.any_instance.expects(:enable_feed_proxy).returns(true) | |
| 188 | - Environment.any_instance.expects(:disable_feed_ssl).returns(false) | |
| 189 | - Environment.any_instance.expects(:http_feed_proxy).twice.returns('http://127.0.0.1:3128') | |
| 186 | + env = Environment.new | |
| 187 | + env.expects(:disable_feed_ssl).returns(false) | |
| 188 | + env.expects(:http_feed_proxy).twice.returns('http://127.0.0.1:3128') | |
| 190 | 189 | |
| 191 | 190 | handler.expects(:open).with('http://site.org/feed.xml', {"User-Agent" => "Noosfero/#{Noosfero::VERSION}", :proxy => 'http://127.0.0.1:3128'}, anything).returns('bli content') |
| 192 | 191 | |
| 193 | - assert_equal 'bli content', handler.fetch('http://site.org/feed.xml') | |
| 192 | + assert_equal 'bli content', handler.fetch_through_proxy('http://site.org/feed.xml', env) | |
| 194 | 193 | end |
| 195 | 194 | |
| 196 | 195 | should 'not verify ssl when define env parameter SSL_VERIFY_NONE' do |
| 197 | - Environment.any_instance.expects(:enable_feed_proxy).returns(false) | |
| 198 | - Environment.any_instance.expects(:disable_feed_ssl).returns(true) | |
| 196 | + env = Environment.new | |
| 197 | + env.expects(:disable_feed_ssl).returns(true) | |
| 199 | 198 | |
| 200 | 199 | handler.expects(:open).with('http://site.org/feed.xml', {"User-Agent" => "Noosfero/#{Noosfero::VERSION}", :ssl_verify_mode => OpenSSL::SSL::VERIFY_NONE}, anything) |
| 201 | 200 | |
| 202 | - handler.fetch('http://site.org/feed.xml') | |
| 201 | + handler.fetch_through_proxy('http://site.org/feed.xml', env) | |
| 202 | + end | |
| 203 | + | |
| 204 | + [:external_feed, :feed_reader_block].each do |container_class| | |
| 205 | + should "not use proxy settings for #{container_class} from one environment on another" do | |
| 206 | + one = create(:environment) | |
| 207 | + one.expects(:enable_feed_proxy).returns(true) | |
| 208 | + container_one = create(container_class) | |
| 209 | + container_one.stubs(:environment).returns(one) | |
| 210 | + | |
| 211 | + handler.expects(:fetch_through_proxy).with(container_one.address, one) | |
| 212 | + handler.process(container_one) | |
| 213 | + | |
| 214 | + another = create(:environment) | |
| 215 | + another.expects(:enable_feed_proxy).returns(false) | |
| 216 | + container_another = create(container_class) | |
| 217 | + container_another.stubs(:environment).returns(another) | |
| 218 | + | |
| 219 | + handler.expects(:fetch_through_proxy).never.with(container_another.address, another) | |
| 220 | + handler.process(container_another) | |
| 221 | + end | |
| 203 | 222 | end |
| 204 | 223 | |
| 205 | 224 | end | ... | ... |