From 7524d52276c367dca5ef0a77ac0ec34f774173c2 Mon Sep 17 00:00:00 2001 From: evazion Date: Sun, 11 Nov 2018 20:18:21 -0600 Subject: [PATCH] Fix #3985: Uploads: 405 Method Not Allowed. --- app/logical/cloudflare_service.rb | 9 +++++++ app/logical/downloads/file.rb | 38 ++++++++++++---------------- test/unit/cloudflare_service_test.rb | 6 +++++ test/unit/downloads/file_test.rb | 13 ++++------ 4 files changed, 36 insertions(+), 30 deletions(-) diff --git a/app/logical/cloudflare_service.rb b/app/logical/cloudflare_service.rb index 76c2e0c31..ed8ca6241 100644 --- a/app/logical/cloudflare_service.rb +++ b/app/logical/cloudflare_service.rb @@ -22,6 +22,15 @@ class CloudflareService }) end + def ips(expiry: 24.hours) + text, code = HttpartyCache.get("https://api.cloudflare.com/client/v4/ips", expiry: expiry) + return [] if code != 200 + + json = JSON.parse(text, symbolize_names: true) + ips = json[:result][:ipv4_cidrs] + json[:result][:ipv6_cidrs] + ips.map { |ip| IPAddr.new(ip) } + end + def delete(md5, ext) url = "https://api.cloudflare.com/client/v4/zones/#{zone}/purge_cache" files = ["#{md5}.#{ext}", "preview/#{md5}.jpg", "sample/sample-#{md5}.jpg"].map do |name| diff --git a/app/logical/downloads/file.rb b/app/logical/downloads/file.rb index 45298969b..e4fa2d128 100644 --- a/app/logical/downloads/file.rb +++ b/app/logical/downloads/file.rb @@ -10,28 +10,6 @@ module Downloads validate :validate_url - # Prevent Cloudflare from potentially mangling the image. See issue #3528. - def uncached_url - url = file_url.dup - - if is_cloudflare? - url.query_values = (url.query_values || {}).merge(danbooru_no_cache: SecureRandom.uuid) - end - - url - end - - def is_cloudflare? - return false if ENV["SKIP_CLOUDFLARE_CHECK"] - - Cache.get("is_cloudflare:#{file_url.origin}", 4.hours) do - res = HTTParty.head(file_url, httparty_options) - raise Error.new("HTTP error code: #{res.code} #{res.message}") unless res.success? - - res.key?("CF-Ray") - end - end - def initialize(url, referer=nil) @url = Addressable::URI.parse(url) rescue nil @referer = referer @@ -79,6 +57,15 @@ module Downloads end end # def + # Prevent Cloudflare from potentially mangling the image. See issue #3528. + def uncached_url + return file_url unless is_cloudflare?(file_url) + + url = file_url.dup + url.query_values = url.query_values.to_h.merge(danbooru_no_cache: SecureRandom.uuid) + url + end + def file_url @file_url ||= Addressable::URI.parse(strategy.image_url) end @@ -95,6 +82,13 @@ module Downloads connection_adapter: ValidatingConnectionAdapter, }.deep_merge(Danbooru.config.httparty_options) end + + def is_cloudflare?(url) + return false if ENV["SKIP_CLOUDFLARE_CHECK"] + + ip_addr = IPAddr.new(Resolv.getaddress(url.hostname)) + CloudflareService.new.ips.any? { |subnet| subnet.include?(ip_addr) } + end end # Hook into HTTParty to validate the IP before following redirects. diff --git a/test/unit/cloudflare_service_test.rb b/test/unit/cloudflare_service_test.rb index ea74ac296..4dd914eda 100644 --- a/test/unit/cloudflare_service_test.rb +++ b/test/unit/cloudflare_service_test.rb @@ -24,4 +24,10 @@ class CloudflareServiceTest < ActiveSupport::TestCase end end end + + context "#ips" do + should "work" do + refute_empty(subject.ips) + end + end end diff --git a/test/unit/downloads/file_test.rb b/test/unit/downloads/file_test.rb index 544790930..243704393 100644 --- a/test/unit/downloads/file_test.rb +++ b/test/unit/downloads/file_test.rb @@ -9,13 +9,15 @@ module Downloads end context "for a banned IP" do + setup do + Resolv.expects(:getaddress).returns("127.0.0.1").at_least_once + end + should "not try to download the file" do - Resolv.expects(:getaddress).returns("127.0.0.1") assert_raise(Downloads::File::Error) { Downloads::File.new("http://evil.com").download! } end should "not try to fetch the size" do - Resolv.expects(:getaddress).returns("127.0.0.1") assert_raise(Downloads::File::Error) { Downloads::File.new("http://evil.com").size } end @@ -29,15 +31,9 @@ module Downloads should "not follow redirects that resolve to a banned IP" do url = "http://httpbin.org/redirect-to?url=http://127.0.0.1.nip.io" stub_request(:get, url).to_return(status: 301, headers: { "Location": "http://127.0.0.1.xip.io" }) - Resolv.expects(:getaddress).returns("127.0.0.1") assert_raise(Downloads::File::Error) { Downloads::File.new(url).download! } end - - should "not send a HEAD request when checking for cloudflare" do - Resolv.expects(:getaddress).with("www.google.com").returns("127.0.0.1") - assert_raise(Downloads::File::Error) { @download.is_cloudflare? } - end end context "that fails" do @@ -52,6 +48,7 @@ module Downloads resp = stub("resp", success?: true) HTTParty.expects(:get).twice.multiple_yields("a", bomb, "b", "c").then.multiple_yields("a", "b", "c").returns(resp) + @download.stubs(:is_cloudflare?).returns(false) tempfile, _ = @download.download! assert_equal("abc", tempfile.read)