Skip to content

Commit

Permalink
1680: add sanitation of error-message from potentially external accou…
Browse files Browse the repository at this point in the history
…nt_link partner
  • Loading branch information
kkoehn committed Feb 5, 2025
1 parent 0f59bfc commit c6ee504
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 14 deletions.
27 changes: 16 additions & 11 deletions app/services/task_service/push_external.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,21 +9,24 @@ def initialize(zip:, account_link:)
end

def execute
body = @zip.string
begin
response = connection.post {|request| request_parameters(request, body) }
if response.success?
nil
else
response.status == 401 ? I18n.t('tasks.export_external_confirm.not_authorized', account_link: @account_link.name) : response.body
end
rescue StandardError => e
e
end
response = connection.post {|request| request_parameters(request, @zip.string) }
return nil if response.success?
return I18n.t('tasks.export_external_confirm.not_authorized', account_link: @account_link.name) if response.status == 401

handle_error(message: response.body)
rescue Faraday::ServerError => e
handle_error(error: e, message: I18n.t('tasks.export_external_confirm.server_error', account_link: @account_link.name))
rescue StandardError => e
handle_error(error: e)
end

private

def handle_error(message: nil, error: nil)
Sentry.capture_exception(error) if error.present?
ERB::Util.html_escape(message || error.to_s)
end

def request_parameters(request, body)
request.tap do |req|
req.headers['Content-Type'] = 'application/zip'
Expand All @@ -36,6 +39,8 @@ def request_parameters(request, body)
def connection
Faraday.new(url: @account_link.push_url) do |faraday|
faraday.adapter Faraday.default_adapter
faraday.options[:open_timeout] = 5
faraday.options[:timeout] = 5
end
end
end
Expand Down
3 changes: 2 additions & 1 deletion config/locales/de/controllers/tasks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,9 @@ de:
duplicate:
error_alert: Die Aufgabe konnte nicht dupliziert werden.
export_external_confirm:
error: 'Der Export der Aufgabe (%{title}) ist fehlgeschlagen. <br> Fehler: %{error}'
error: 'Der Export der Aufgabe (%{title}) ist fehlgeschlagen. <br><br> Fehler: %{error}'
not_authorized: Die Autorisierung mit "%{account_link}" konnte nicht hergestellt werden. Ist der API-Schlüssel korrekt?
server_error: Verbindung zu %{account_link} fehlgeschlagen. Gegenseite nicht erreichbar.
success: Aufgabe (%{title}) erfolgreich exportiert.
import:
internal_error: Beim Import dieser Aufgabe ist auf CodeHarbor ein interner Fehler aufgetreten.
Expand Down
3 changes: 2 additions & 1 deletion config/locales/en/controllers/tasks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,9 @@ en:
duplicate:
error_alert: Task could not be duplicated
export_external_confirm:
error: 'Export of task (%{title}) failed. <br> Error: %{error}'
error: 'Export of task (%{title}) failed. <br><br> Error: %{error}'
not_authorized: Authorization with could not be established with "%{account_link}". Is the API Key correct?
server_error: Connection to %{account_link} failed. Remote host unreachable.
success: Task (%{title}) successfully exported.
import:
internal_error: An internal error occurred on CodeHarbor while importing the exercise.
Expand Down
26 changes: 25 additions & 1 deletion spec/services/task_service/push_external_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,13 @@
let(:status) { 500 }
let(:response) { 'an error occured' }

it { is_expected.to be response }
it { is_expected.to eql response }

context 'when response contains problematic characters' do
let(:response) { 'an <error> occurred' }

it { is_expected.to eql 'an &lt;error&gt; occurred' }
end
end

context 'when response status is 401' do
Expand All @@ -60,6 +66,24 @@

it { is_expected.to eq response }
end

context 'when faraday throws an error' do
let(:connection) { instance_double(Faraday::Connection) }
let(:error) { Faraday::ServerError }

before do
allow(Faraday).to receive(:new).and_return(connection)
allow(connection).to receive(:post).and_raise(error)
end

it { is_expected.to eql I18n.t('tasks.export_external_confirm.server_error', account_link: account_link.name) }

context 'when another error occurs' do
let(:error) { 'another error' }

it { is_expected.to eql 'another error' }
end
end
end

context 'when an error occurs' do
Expand Down

0 comments on commit c6ee504

Please sign in to comment.