Skip to content

Commit

Permalink
Ensure Location header on redirects before parsing
Browse files Browse the repository at this point in the history
Fixes crash when HTTP redirect received without a Location header set.
  • Loading branch information
gregose committed Aug 8, 2013
1 parent 093b380 commit 6a992de
Show file tree
Hide file tree
Showing 7 changed files with 47 additions and 6 deletions.
3 changes: 2 additions & 1 deletion Gemfile
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
source 'https://rubygems.org'
gem 'rest-client', '~>1.3'
gem 'addressable', '~>2.3'
gem 'addressable', '~>2.3'
gem 'thin'
8 changes: 8 additions & 0 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,21 @@ GEM
remote: https://rubygems.org/
specs:
addressable (2.3.4)
daemons (1.1.9)
eventmachine (1.0.3)
mime-types (1.23)
rack (1.5.2)
rest-client (1.6.7)
mime-types (>= 1.16)
thin (1.5.1)
daemons (>= 1.0.9)
eventmachine (>= 0.12.6)
rack (>= 1.0.0)

PLATFORMS
ruby

DEPENDENCIES
addressable (~> 2.3)
rest-client (~> 1.3)
thin
12 changes: 11 additions & 1 deletion Rakefile
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,22 @@ end
task :build => 'server.js'

namespace :test do
desc "Start test server"
task :server do |t|
$SERVER_PID = Process.spawn("ruby test/proxy_test_server.rb")
end

desc "Run the tests against localhost"
task :check do |t|
system("ruby test/proxy_test.rb")
end

desc "Kill test server"
task :kill_server do |t|
Process.kill(:QUIT, $SERVER_PID) && Process.wait
end
end
task :default => [:build, "test:check"]
task :default => [:build, "test:server", "test:check", "test:kill_server"]

Dir["tasks/*.rake"].each do |f|
load f
Expand Down
2 changes: 2 additions & 0 deletions server.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,8 @@ process_url = (url, transferred_headers, resp, remaining_redirects) ->
when 301, 302, 303, 307
if remaining_redirects <= 0
four_oh_four(resp, "Exceeded max depth")
else if !srcResp.headers['location']
four_oh_four(resp, "Redirect with no location")
else
is_finished = false
newUrl = Url.parse srcResp.headers['location']
Expand Down
8 changes: 4 additions & 4 deletions server.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 9 additions & 0 deletions test/proxy_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
require 'openssl'
require 'rest_client'
require 'addressable/uri'
require 'thin'

require 'test/unit'

Expand All @@ -13,6 +14,14 @@ def config
'host' => ENV['CAMO_HOST'] || "http://localhost:8081" }
end

def test_proxy_survives_redirect_without_location
assert_raise RestClient::ResourceNotFound do
request('http://localhost:9292')
end
response = request('http://media.ebaumsworld.com/picture/Mincemeat/Pimp.jpg')
assert_equal(200, response.code)
end

def test_proxy_valid_image_url
response = request('http://media.ebaumsworld.com/picture/Mincemeat/Pimp.jpg')
assert_equal(200, response.code)
Expand Down
11 changes: 11 additions & 0 deletions test/proxy_test_server.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
require 'thin'

class ProxyTestServer
def call(env)
[302, {"Content-Type" => "image/foo"}, "test"]
end
end

Thin::Server.start('127.0.0.1', 9292) do
run ProxyTestServer.new
end

0 comments on commit 6a992de

Please sign in to comment.