Skip to content

Commit

Permalink
Merge pull request #537 from jvlcek/bz_1661445_error_w_sql
Browse files Browse the repository at this point in the history
Remove SQL select from exception error messages.

(cherry picked from commit 325a5a1)

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1686021
  • Loading branch information
gtanzillo authored and simaishi committed Mar 6, 2019
1 parent 570ac58 commit 9319501
Show file tree
Hide file tree
Showing 3 changed files with 71 additions and 3 deletions.
4 changes: 2 additions & 2 deletions app/controllers/api/base_controller/authentication.rb
Expand Up @@ -43,9 +43,9 @@ def require_api_user_or_token
response.headers["Content-Type"] = "application/json"
case auth_mechanism
when :system, :token
render :status => 401, :json => ErrorSerializer.new(:unauthorized, e).serialize.to_json
render :status => 401, :json => ErrorSerializer.new(:unauthorized, e).serialize(true).to_json
when :basic, nil
request_http_basic_authentication("Application", ErrorSerializer.new(:unauthorized, e).serialize.to_json)
request_http_basic_authentication("Application", ErrorSerializer.new(:unauthorized, e).serialize(true).to_json)
end
log_api_response
end
Expand Down
12 changes: 11 additions & 1 deletion lib/services/api/error_serializer.rb
Expand Up @@ -2,21 +2,31 @@ module Api
class ErrorSerializer
attr_reader :kind, :error

SQL_STATEMENTS = " SELECT | UPDATE | INSERT | DELETE ".freeze

def initialize(kind, error)
@kind = kind
@error = error
end

def serialize
def serialize(no_sql_query = false)
result = {
:error => {
:kind => kind,
:message => error.message,
:klass => error.class.name
}
}
result[:error][:message] = remove_sql_query(error.message) if no_sql_query
result[:error][:backtrace] = error.backtrace.join("\n") if Rails.env.test?
result
end

private

def remove_sql_query(message)
return message unless message =~ /PG.*ERROR/i
message.split(/#{SQL_STATEMENTS}/i)[0]
end
end
end
58 changes: 58 additions & 0 deletions spec/lib/services/api/error_serializer_spec.rb
@@ -0,0 +1,58 @@
RSpec.describe Api::ErrorSerializer do
let(:non_sql_message) { "Not a PostgreSQL error" }
let(:non_sql_error) { double("non sql error", :message => non_sql_message, :backtrace => []) }

let(:pg_message_header) { "PG::Error" }

let(:pg_select_error) { "#{pg_message_header} SeLeCt \"users\".* FROM \"users\"" }
let(:select_error) { double("select error", :message => pg_select_error, :backtrace => []) }

let(:pg_update_error) { "#{pg_message_header} UpDate \"users\".* FROM \"users\"" }
let(:update_error) { double("update error", :message => pg_update_error, :backtrace => []) }

let(:pg_insert_error) { "#{pg_message_header} Insert \"users\".* FROM \"users\"" }
let(:insert_error) { double("insert error", :message => pg_insert_error, :backtrace => []) }

let(:delete_error) { double("delete error", :message => pg_delete_error, :backtrace => []) }
let(:pg_delete_error) { "#{pg_message_header} dEleTe \"users\".* FROM \"users\"" }

let(:kind) { "kind" }

describe ".serialize" do
it "returns the original message when not a PG error" do
actual = described_class.new(kind, non_sql_error).serialize

expect(actual[:error][:message]).to eq(non_sql_message)
end

it "returns a message with the SQL statement by default" do
actual = described_class.new(kind, select_error).serialize

expect(actual[:error][:message]).to eq(pg_select_error)
end

it "returns a message without the SQL SELECT statement when requested" do
actual = described_class.new(kind, select_error).serialize(true)

expect(actual[:error][:message]).to eq(pg_message_header)
end

it "returns a message without the SQL UPDATE statement when requested" do
actual = described_class.new(kind, update_error).serialize(true)

expect(actual[:error][:message]).to eq(pg_message_header)
end

it "returns a message without the SQL INSERT statement when requested" do
actual = described_class.new(kind, insert_error).serialize(true)

expect(actual[:error][:message]).to eq(pg_message_header)
end

it "returns a message without the SQL DELETE statement when requested" do
actual = described_class.new(kind, delete_error).serialize(true)

expect(actual[:error][:message]).to eq(pg_message_header)
end
end
end

0 comments on commit 9319501

Please sign in to comment.