diff --git a/lib/travis/api/app/endpoint/jobs.rb b/lib/travis/api/app/endpoint/jobs.rb index 0c497a58..302112e0 100644 --- a/lib/travis/api/app/endpoint/jobs.rb +++ b/lib/travis/api/app/endpoint/jobs.rb @@ -3,6 +3,8 @@ require 'travis/api/app' class Travis::Api::App class Endpoint class Jobs < Endpoint + include Helpers::Accept + get '/' do prefer_follower do respond_with service(:find_jobs, params) @@ -49,15 +51,21 @@ class Travis::Api::App get '/:job_id/log' do resource = service(:find_log, params).run - if !resource || resource.archived? - archived_log_path = archive_url("/jobs/#{params[:job_id]}/log.txt") + if (!resource || resource.archived?) + # the way we use responders makes it hard to validate proper format + # automatically here, so we need to check it explicitly + if accepts?('text/plain') + archived_log_path = archive_url("/jobs/#{params[:job_id]}/log.txt") - if params[:cors_hax] - status 204 - headers['Access-Control-Expose-Headers'] = 'Location' - headers['Location'] = archived_log_path + if params[:cors_hax] + status 204 + headers['Access-Control-Expose-Headers'] = 'Location' + headers['Location'] = archived_log_path + else + redirect archived_log_path, 307 + end else - redirect archived_log_path, 307 + status 406 end else respond_with resource diff --git a/lib/travis/api/app/helpers/accept.rb b/lib/travis/api/app/helpers/accept.rb index a57dfa8e..229f544e 100644 --- a/lib/travis/api/app/helpers/accept.rb +++ b/lib/travis/api/app/helpers/accept.rb @@ -64,6 +64,10 @@ class Travis::Api::App end end + def accepts?(mime_type) + accept_entries.any? { |e| e.accepts?(mime_type) } + end + def accept_entries entries = env['HTTP_ACCEPT'].to_s.delete(' ').to_s.split(',').map { |e| Entry.new(e) } entries.empty? ? [Entry.new('*/*')] : entries.sort diff --git a/spec/integration/v2/jobs_spec.rb b/spec/integration/v2/jobs_spec.rb index 13be7a25..1183d619 100644 --- a/spec/integration/v2/jobs_spec.rb +++ b/spec/integration/v2/jobs_spec.rb @@ -28,6 +28,7 @@ describe 'Jobs' do context 'when log is archived' do it 'redirects to archive' do job.log.update_attributes!(content: 'the log', archived_at: Time.now, archive_verified: true) + headers = { 'HTTP_ACCEPT' => 'text/plain; version=2' } response = get "/jobs/#{job.id}/log.txt", {}, headers response.should redirect_to("https://s3.amazonaws.com/archive.travis-ci.org/jobs/#{job.id}/log.txt") end @@ -36,6 +37,7 @@ describe 'Jobs' do context 'when log is missing' do it 'redirects to archive' do job.log.destroy + headers = { 'HTTP_ACCEPT' => 'text/plain; version=2' } response = get "/jobs/#{job.id}/log.txt", {}, headers response.should redirect_to("https://s3.amazonaws.com/archive.travis-ci.org/jobs/#{job.id}/log.txt") end @@ -44,6 +46,7 @@ describe 'Jobs' do context 'with cors_hax param' do it 'renders No Content response with location of the archived log' do job.log.destroy + headers = { 'HTTP_ACCEPT' => 'text/plain; version=2' } response = get "/jobs/#{job.id}/log.txt?cors_hax=true", {}, headers response.status.should == 204 response.headers['Location'].should == "https://s3.amazonaws.com/archive.travis-ci.org/jobs/#{job.id}/log.txt" @@ -76,8 +79,10 @@ describe 'Jobs' do end it 'responds with 406 when log is already aggregated' do - job.log.update_attributes(aggregated_at: Time.now) - headers = { 'HTTP_ACCEPT' => 'application/vnd.travis-ci.2+json; chunked=true' } + job.log.update_attributes(aggregated_at: Time.now, archived_at: Time.now, archive_verified: true) + job.log.should be_archived + + headers = { 'HTTP_ACCEPT' => 'application/json; version=2; chunked=true' } response = get "/jobs/#{job.id}/log", {}, headers response.status.should == 406 end