From 81bac0b09b2d03de76c8885f6d77c9297a3ead34 Mon Sep 17 00:00:00 2001
From: Anton Khorev <tony29@yandex.ru>
Date: Thu, 19 Sep 2024 18:38:50 +0300
Subject: [PATCH] Don't include stack backtraces on xml errors when importing
 traces

---
 app/jobs/trace_importer_job.rb       |   4 ++++
 test/gpx/fixtures/jpg.gpx            | Bin 0 -> 287 bytes
 test/gpx/fixtures/jpg.gz.gpx         | Bin 0 -> 179 bytes
 test/jobs/trace_importer_job_test.rb |  32 +++++++++++++++++++++++++++
 4 files changed, 36 insertions(+)
 create mode 100644 test/gpx/fixtures/jpg.gpx
 create mode 100644 test/gpx/fixtures/jpg.gz.gpx

diff --git a/app/jobs/trace_importer_job.rb b/app/jobs/trace_importer_job.rb
index 940dd6c79..48285cbc1 100644
--- a/app/jobs/trace_importer_job.rb
+++ b/app/jobs/trace_importer_job.rb
@@ -10,6 +10,10 @@ class TraceImporterJob < ApplicationJob
       UserMailer.gpx_failure(trace, "0 points parsed ok. Do they all have lat,lng,alt,timestamp?").deliver
       trace.destroy
     end
+  rescue XML::Error => e
+    logger.info e.to_s
+    UserMailer.gpx_failure(trace, e).deliver
+    trace.destroy
   rescue StandardError => e
     logger.info e.to_s
     e.backtrace.each { |l| logger.info l }
diff --git a/test/gpx/fixtures/jpg.gpx b/test/gpx/fixtures/jpg.gpx
new file mode 100644
index 0000000000000000000000000000000000000000..12dc8c4ab9307fcf44c9a6bafc0b64745eb98fcb
GIT binary patch
literal 287
zcmb7;$qm9V5JkUZ4_R!7nP7($ksfkDLV_x&k19A&1j~g^jKd9b;_08B{w(R1Uf|>Y
zu!qWwY6Q{~JLsaAlQB6|5g9{GS*|MAIA<GcecQ2brL~rOmQ9E;y7hMG!_b8o<|Rt!
z8ACNu9lZ77Z_)))fx%h{3Q<~Ax+3OR@lBm|;9rZ`F1jBp%kBeCt9(TZCY(>el<sd*
CrXS}3

literal 0
HcmV?d00001

diff --git a/test/gpx/fixtures/jpg.gz.gpx b/test/gpx/fixtures/jpg.gz.gpx
new file mode 100644
index 0000000000000000000000000000000000000000..2742808e1274dfb685909d0c24a1dee053ee3423
GIT binary patch
literal 179
zcmV;k08IZMiwFpBbM0mT18Q(*E@yCf0F{o(4Z<)GMZaSYS!{=yV22cu9&$iJf-0zw
zDmYLC%Y{yi!wquc>7SndEa{eB;N$+Vhsum<1kw{b=%SdDF*#Hb8ADB3t}53!XB%sM
z+p%w@wU&F9O^7kN^>*mP(1jT0B}(TRLp4zyy!GL4(gjk1!CDClQCd{GBIZ}|O`Ub%
hUyIo;x*sb``+(CbU(tdI=Mylc`x`AWJ31c$005V-Pf`E?

literal 0
HcmV?d00001

diff --git a/test/jobs/trace_importer_job_test.rb b/test/jobs/trace_importer_job_test.rb
index e1ddcc967..c6d44ecce 100644
--- a/test/jobs/trace_importer_job_test.rb
+++ b/test/jobs/trace_importer_job_test.rb
@@ -52,6 +52,38 @@ class TraceImporterJobTest < ActiveJob::TestCase
     email = ActionMailer::Base.deliveries.last
     assert_equal trace.user.email, email.to[0]
     assert_match(/failure/, email.subject)
+    assert_no_match(/Start tag expected/, email.text_part.body.to_s, "should not include parser error")
+    assert_match(%r{jobs/trace_importer_job\.rb}, email.text_part.body.to_s, "should include stack backtrace")
+
+    ActionMailer::Base.deliveries.clear
+  end
+
+  def test_parse_error_notification
+    trace = create(:trace, :inserted => false, :fixture => "jpg")
+    Rails.logger.silence do
+      TraceImporterJob.perform_now(trace)
+    end
+
+    email = ActionMailer::Base.deliveries.last
+    assert_equal trace.user.email, email.to[0]
+    assert_match(/failure/, email.subject)
+    assert_match(/Start tag expected/, email.text_part.body.to_s, "should include parser error")
+    assert_no_match(%r{jobs/trace_importer_job\.rb}, email.text_part.body.to_s, "should not include stack backtrace")
+
+    ActionMailer::Base.deliveries.clear
+  end
+
+  def test_gz_parse_error_notification
+    trace = create(:trace, :inserted => false, :fixture => "jpg.gz")
+    Rails.logger.silence do
+      TraceImporterJob.perform_now(trace)
+    end
+
+    email = ActionMailer::Base.deliveries.last
+    assert_equal trace.user.email, email.to[0]
+    assert_match(/failure/, email.subject)
+    assert_match(/Start tag expected/, email.text_part.body.to_s, "should include parser error")
+    assert_no_match(%r{jobs/trace_importer_job\.rb}, email.text_part.body.to_s, "should not include stack backtrace")
 
     ActionMailer::Base.deliveries.clear
   end
-- 
2.39.5