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

Change FormatParser.parse_http to follow a redirect #190

Merged
merged 5 commits into from
Feb 17, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions format_parser.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ Gem::Specification.new do |spec|
spec.add_dependency 'exifr', '~> 1', '>= 1.3.8'
spec.add_dependency 'id3tag', '~> 0.14'
spec.add_dependency 'faraday', '~> 0.13'
spec.add_dependency 'faraday_middleware', '~> 0.14'
spec.add_dependency 'measurometer', '~> 1'

spec.add_development_dependency 'rspec', '~> 3.0'
Expand Down
7 changes: 6 additions & 1 deletion lib/remote_io.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ class InvalidRequest < UpstreamError
# @param uri[URI, String] the remote URL to obtain
def initialize(uri)
require 'faraday'
require 'faraday_middleware/response/follow_redirects'
@uri = uri
@pos = 0
@remote_size = false
Expand Down Expand Up @@ -78,7 +79,11 @@ def request_range(range)
# We use a GET and not a HEAD request followed by a GET because
# S3 does not allow HEAD requests if you only presigned your URL for GETs, so we
# combine the first GET of a segment and retrieving the size of the resource
response = Faraday.get(@uri, nil, range: 'bytes=%d-%d' % [range.begin, range.end])
conn = Faraday.new do |faraday|
faraday.use FaradayMiddleware::FollowRedirects
faraday.adapter Faraday.default_adapter
end
response = conn.get(@uri, nil, range: 'bytes=%d-%d' % [range.begin, range.end])

case response.status
when 200, 206
Expand Down
11 changes: 11 additions & 0 deletions spec/remote_fetching_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@
}
@server = WEBrick::HTTPServer.new(options)
@server.mount '/', WEBrick::HTTPServlet::FileHandler, fixtures_dir
@server.mount_proc '/redirect' do |req, res|
res.status = 302
res.header['Location'] = req.path.sub('/redirect', '')
end
trap('INT') { @server.stop }
@server_thread = Thread.new { @server.start }
end
Expand Down Expand Up @@ -91,6 +95,13 @@
end
end

context 'when the server respond with a redirect' do
it 'follows the redirect' do
file_information = FormatParser.parse_http('http://localhost:9399/redirect/TIFF/test.tif')
expect(file_information.format).to eq(:tif)
end
end

after(:all) do
@server.stop
@server_thread.join(0.5)
Expand Down
35 changes: 22 additions & 13 deletions spec/remote_io_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
rio = described_class.new('https://images.invalid/img.jpg')

fake_resp = double(headers: {'Content-Range' => '10-109/2577'}, status: 206, body: 'This is the response')
expect(Faraday).to receive(:get).with('https://images.invalid/img.jpg', nil, range: 'bytes=10-109').and_return(fake_resp)
expect_any_instance_of(Faraday::Connection).to receive(:get).with('https://images.invalid/img.jpg', nil, range: 'bytes=10-109').and_return(fake_resp)

rio.seek(10)
read_result = rio.read(100)
Expand All @@ -18,7 +18,7 @@
rio = described_class.new('https://images.invalid/img.jpg')

fake_resp = double(headers: {'Content-Range' => '10-109/2577'}, status: 200, body: 'This is the response')
expect(Faraday).to receive(:get).with('https://images.invalid/img.jpg', nil, range: 'bytes=10-109').and_return(fake_resp)
expect_any_instance_of(Faraday::Connection).to receive(:get).with('https://images.invalid/img.jpg', nil, range: 'bytes=10-109').and_return(fake_resp)

rio.seek(10)
read_result = rio.read(100)
Expand All @@ -29,7 +29,7 @@
rio = described_class.new('https://images.invalid/img.jpg')

fake_resp = double(headers: {}, status: 403, body: 'Please log in')
expect(Faraday).to receive(:get).with('https://images.invalid/img.jpg', nil, range: 'bytes=100-199').and_return(fake_resp)
expect_any_instance_of(Faraday::Connection).to receive(:get).with('https://images.invalid/img.jpg', nil, range: 'bytes=100-199').and_return(fake_resp)

rio.seek(100)
expect { rio.read(100) }.to raise_error(/replied with a 403 and refused/)
Expand All @@ -39,7 +39,7 @@
rio = described_class.new('https://images.invalid/img.jpg')

fake_resp = double(headers: {}, status: 416, body: 'You stepped off the ledge of the range')
expect(Faraday).to receive(:get).with('https://images.invalid/img.jpg', nil, range: 'bytes=100-199').and_return(fake_resp)
expect_any_instance_of(Faraday::Connection).to receive(:get).with('https://images.invalid/img.jpg', nil, range: 'bytes=100-199').and_return(fake_resp)

rio.seek(100)
expect(rio.read(100)).to be_nil
Expand All @@ -49,7 +49,7 @@
rio = described_class.new('https://images.invalid/img.jpg')

fake_resp = double(headers: {}, status: 403, body: 'Please log in')
expect(Faraday).to receive(:get).with('https://images.invalid/img.jpg', nil, range: 'bytes=100-199').and_return(fake_resp)
expect_any_instance_of(Faraday::Connection).to receive(:get).with('https://images.invalid/img.jpg', nil, range: 'bytes=100-199').and_return(fake_resp)

rio.seek(100)
# rubocop: disable Lint/AmbiguousBlockAssociation
Expand All @@ -60,7 +60,7 @@
rio = described_class.new('https://images.invalid/img.jpg')

fake_resp = double(headers: {}, status: 416, body: 'You jumped off the end of the file maam')
expect(Faraday).to receive(:get).with('https://images.invalid/img.jpg', nil, range: 'bytes=100-199').and_return(fake_resp)
expect_any_instance_of(Faraday::Connection).to receive(:get).with('https://images.invalid/img.jpg', nil, range: 'bytes=100-199').and_return(fake_resp)

rio.seek(100)
expect(rio.read(100)).to be_nil
Expand All @@ -69,15 +69,24 @@
it 'does not overwrite size when the range cannot be satisfied and the response is 416' do
rio = described_class.new('https://images.invalid/img.jpg')

fake_resp = double(headers: {'Content-Range' => 'bytes 0-0/13'}, status: 206, body: 'a')
expect(Faraday).to receive(:get).with('https://images.invalid/img.jpg', nil, range: 'bytes=0-0').and_return(fake_resp)
fake_resp1 = double(headers: {'Content-Range' => 'bytes 0-0/13'}, status: 206, body: 'a')
fake_resp2 = double(headers: {}, status: 416, body: 'You jumped off the end of the file maam')

faraday_conn = instance_double(Faraday::Connection)
allow(Faraday).to receive(:new).and_return(faraday_conn)
expect(faraday_conn).to receive(:get)
.with('https://images.invalid/img.jpg', nil, range: 'bytes=0-0')
.ordered
.and_return(fake_resp1)
expect(faraday_conn).to receive(:get)
.with('https://images.invalid/img.jpg', nil, range: 'bytes=100-199')
.ordered
.and_return(fake_resp2)

rio.read(1)

expect(rio.size).to eq(13)

fake_resp = double(headers: {}, status: 416, body: 'You jumped off the end of the file maam')
expect(Faraday).to receive(:get).with('https://images.invalid/img.jpg', nil, range: 'bytes=100-199').and_return(fake_resp)

rio.seek(100)
expect(rio.read(100)).to be_nil

Expand All @@ -88,7 +97,7 @@
rio = described_class.new('https://images.invalid/img.jpg')

fake_resp = double(headers: {}, status: 502, body: 'Guru meditation')
expect(Faraday).to receive(:get).with('https://images.invalid/img.jpg', nil, range: 'bytes=100-199').and_return(fake_resp)
expect_any_instance_of(Faraday::Connection).to receive(:get).with('https://images.invalid/img.jpg', nil, range: 'bytes=100-199').and_return(fake_resp)

rio.seek(100)
expect { rio.read(100) }.to raise_error(/replied with a 502 and we might want to retry/)
Expand All @@ -100,7 +109,7 @@
expect(rio.pos).to eq(0)

fake_resp = double(headers: {'Content-Range' => 'bytes 0-0/13'}, status: 206, body: 'a')
expect(Faraday).to receive(:get).with('https://images.invalid/img.jpg', nil, range: 'bytes=0-0').and_return(fake_resp)
expect_any_instance_of(Faraday::Connection).to receive(:get).with('https://images.invalid/img.jpg', nil, range: 'bytes=0-0').and_return(fake_resp)
rio.read(1)

expect(rio.pos).to eq(1)
Expand Down