From 1b8f79123b3355a2876b2f35b028c59a2bd21900 Mon Sep 17 00:00:00 2001 From: Daniel O <86343144+doconnor-clintel@users.noreply.github.com> Date: Fri, 16 May 2025 13:54:11 +0930 Subject: [PATCH 1/4] Avoid crash on nil/undefined variable Fix https://github.com/zvory/csv-safe/issues/15 --- lib/csv-safe.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/csv-safe.rb b/lib/csv-safe.rb index 2a799a7..83d93a3 100644 --- a/lib/csv-safe.rb +++ b/lib/csv-safe.rb @@ -53,7 +53,7 @@ def sanitize_row(row) when self.class::Row then row.fields.map { |field| sanitize_field(field) } when Hash - then @headers.map { |header| sanitize_field(row[header]) } + then headers.map { |header| sanitize_field(row[header]) } else row.map { |field| sanitize_field(field) } end From 3985ebc85ff8f15997aff4f0c42f6ac8ff93c100 Mon Sep 17 00:00:00 2001 From: Daniel O <86343144+doconnor-clintel@users.noreply.github.com> Date: Fri, 16 May 2025 04:31:46 +0000 Subject: [PATCH 2/4] Refactor style --- lib/csv-safe.rb | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/lib/csv-safe.rb b/lib/csv-safe.rb index 83d93a3..4e19476 100644 --- a/lib/csv-safe.rb +++ b/lib/csv-safe.rb @@ -49,13 +49,10 @@ def sanitize_field(field) end def sanitize_row(row) - case row - when self.class::Row - then row.fields.map { |field| sanitize_field(field) } - when Hash - then headers.map { |header| sanitize_field(row[header]) } - else - row.map { |field| sanitize_field(field) } - end + return row.fields.map { |field| sanitize_field(field) } if row.is_a?(self.class::Row) + + return @headers.map { |header| sanitize_field(row[header]) } if row.is_a?(Hash) && !@headers.nil? + + row.map { |field| sanitize_field(field) } end end From b92cd77fa4981096a23fcb1af987c0ab76775d09 Mon Sep 17 00:00:00 2001 From: Daniel O <86343144+doconnor-clintel@users.noreply.github.com> Date: Fri, 16 May 2025 04:38:07 +0000 Subject: [PATCH 3/4] Add more of an end to end test --- spec/csv_safe_spec.rb | 29 ++++++++++++++++++++++++----- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/spec/csv_safe_spec.rb b/spec/csv_safe_spec.rb index fba9f47..7d283e0 100644 --- a/spec/csv_safe_spec.rb +++ b/spec/csv_safe_spec.rb @@ -122,12 +122,12 @@ def self.to_s end describe '#sanitize_row' do - before(:all) do - CSV_Instance = CSVSafe.new('') - end - subject { CSV_Instance.send(:sanitize_row, row) } - context 'when the row is a CSV::Row' do + before(:all) do + CSV_Instance = CSVSafe.new('') + end + subject { CSV_Instance.send(:sanitize_row, row) } + context "when the fields don't require sanitization" do let(:fields) { %w[Jane 30] } let(:row) { CSV::Row.new(%w[Name Age], fields) } @@ -143,6 +143,11 @@ def self.to_s end context 'when the row is a Hash' do + before(:all) do + CSV_Instance = CSVSafe.new('') + end + subject { CSV_Instance.send(:sanitize_row, row) } + before do CSV_Instance.instance_variable_set(:@headers, %i[Name Age]) end @@ -158,9 +163,23 @@ def self.to_s it { should eq expected } end + + it "does not crash" do + headers = %i[a b c] + payload = { b: :b, a: :a, c: :c } + + output = CSVSafe.generate(headers: true) { |csv| csv << headers; csv << payload } + + expect(output).to eq("a,b,c\n\"[:b, :b]\",\"[:a, :a]\",\"[:c, :c]\"\n") + end end context 'when the row is an array' do + before(:all) do + CSV_Instance = CSVSafe.new('') + end + subject { CSV_Instance.send(:sanitize_row, row) } + context "when the fields don't require sanitization" do let(:row) { %w[Jane 30] } it { should eq row } From 4217c187e1ca75280302f775810cc87dda1e2388 Mon Sep 17 00:00:00 2001 From: Daniel O <86343144+doconnor-clintel@users.noreply.github.com> Date: Fri, 16 May 2025 04:40:24 +0000 Subject: [PATCH 4/4] Fix tests to avoid accessing internal variable --- lib/csv-safe.rb | 2 +- spec/csv_safe_spec.rb | 9 +++------ 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/lib/csv-safe.rb b/lib/csv-safe.rb index 4e19476..26db103 100644 --- a/lib/csv-safe.rb +++ b/lib/csv-safe.rb @@ -51,7 +51,7 @@ def sanitize_field(field) def sanitize_row(row) return row.fields.map { |field| sanitize_field(field) } if row.is_a?(self.class::Row) - return @headers.map { |header| sanitize_field(row[header]) } if row.is_a?(Hash) && !@headers.nil? + return headers.map { |header| sanitize_field(row[header]) } if row.is_a?(Hash) && !headers.nil? row.map { |field| sanitize_field(field) } end diff --git a/spec/csv_safe_spec.rb b/spec/csv_safe_spec.rb index 7d283e0..68f427e 100644 --- a/spec/csv_safe_spec.rb +++ b/spec/csv_safe_spec.rb @@ -144,13 +144,10 @@ def self.to_s context 'when the row is a Hash' do before(:all) do - CSV_Instance = CSVSafe.new('') + CSV_Instance = CSVSafe.new('', headers: %i[Name Age]) end subject { CSV_Instance.send(:sanitize_row, row) } - before do - CSV_Instance.instance_variable_set(:@headers, %i[Name Age]) - end context "when the fields don't require sanitization" do let(:row) { { Name: 'Jane', Age: '30' } } let(:expected) { %w[Jane 30] } @@ -168,9 +165,9 @@ def self.to_s headers = %i[a b c] payload = { b: :b, a: :a, c: :c } - output = CSVSafe.generate(headers: true) { |csv| csv << headers; csv << payload } + output = CSVSafe.generate(headers: true) { |csv| csv << headers; csv << payload }.split "\n" - expect(output).to eq("a,b,c\n\"[:b, :b]\",\"[:a, :a]\",\"[:c, :c]\"\n") + expect(output).to eq(["a,b,c", "a,b,c"]) end end