From be666720a31c4d7769c5d287d06d0a5e4f52b2e8 Mon Sep 17 00:00:00 2001 From: Anton Petrunich Date: Mon, 21 Nov 2016 11:52:46 +0200 Subject: [PATCH 1/3] Allow to ignore invalid fields In some cases you do not want to loose all data if you have only one invalid field in encoded VCard (especially for multi-contact vcards). This change add two config parameters ::Vcard.configuration.raise_on_invalid_line (default true). When this parameter is set to true, decode will raise ::Vcard::InvalidEncodingError if found a field that encoded incorrectly (default behavior). When raise_on_invalid_line = false, exception will not be raised but field will be marked as invalid. With parameter ::Vcard.configuration.ignore_invalid_vcards you can define behavior for invalid fields. If ignore_invalid_vcards = true, then ::Vcard.decode will ignore a vcard that have at least one incorrect field. With ignore_invalid_vcards = false, vcard only invalid fields will be ignored, but vcard will be added to list, returned by .decode --- README.md | 19 ++++++++++++++++++ lib/vcard.rb | 10 ++++++++++ lib/vcard/configuration.rb | 19 ++++++++++++++++++ lib/vcard/dirinfo.rb | 17 +++++++++++++--- lib/vcard/field.rb | 24 ++++++++++++++++------- lib/vcard/vcard.rb | 3 ++- test/field_test.rb | 40 +++++++++++++++++++++++++------------- test/vcard_test.rb | 40 +++++++++++++++++++++++++++++++++++++- 8 files changed, 146 insertions(+), 26 deletions(-) create mode 100644 lib/vcard/configuration.rb diff --git a/README.md b/README.md index 73a97b0..00d958c 100644 --- a/README.md +++ b/README.md @@ -16,6 +16,25 @@ Or install it yourself as: $ gem install vcard +## Configuration + +You can configure how to make deal with invalid lines. +Gem supports three behaviors: +1. `raise_on_invalid_line = true` + Vcard::InvalidEncodingError will be races if any invalid line will be found + +2. `raise_on_invalid_line = false, ignore_invalid_vcards = true` + If vcard source has invalid line, then this vcard object will be ignored. + In case if you have only one vcard object in your source string, empty array will be returned from `Vcard.decode` + +3. `raise_on_invalid_line = false, ignore_invalid_vcards = false` + vcard will be marked as invalid, invalid field will be ignored, but vcard will be present in `Vcard#decode` results + + Vcard.configure do |config| + config.raise_on_invalid_line = false # default true + config.ignore_invalid_vcards = false # default true + end + ## Upgrade Notes We are no longer testing against Ruby 1.8.7. diff --git a/lib/vcard.rb b/lib/vcard.rb index d1be010..960670a 100644 --- a/lib/vcard.rb +++ b/lib/vcard.rb @@ -7,6 +7,7 @@ require "open-uri" require "stringio" +require "vcard/configuration" require "vcard/attachment" require "vcard/bnf" require "vcard/dirinfo" @@ -285,4 +286,13 @@ def self.outer_inner(fields) #:nodoc: end return outer, inner end + + def configuration + @configuration ||= Configuration.new + end + + def configure + yield configuration + end + end diff --git a/lib/vcard/configuration.rb b/lib/vcard/configuration.rb new file mode 100644 index 0000000..99556d3 --- /dev/null +++ b/lib/vcard/configuration.rb @@ -0,0 +1,19 @@ +module Vcard + class Configuration + + attr_accessor :raise_on_invalid_line + alias_method :raise_on_invalid_line?, :raise_on_invalid_line + + attr_accessor :ignore_invalid_vcards + alias_method :ignore_invalid_vcards?, :ignore_invalid_vcards + + def initialize + @raise_on_invalid = true + @ignore_invalid_vcard = true + end + + private + + + end +end \ No newline at end of file diff --git a/lib/vcard/dirinfo.rb b/lib/vcard/dirinfo.rb index 99d60b6..9037017 100644 --- a/lib/vcard/dirinfo.rb +++ b/lib/vcard/dirinfo.rb @@ -31,16 +31,27 @@ class DirectoryInfo # Initialize a DirectoryInfo object from +fields+. If +profile+ is # specified, check the BEGIN/END fields. def initialize(fields, profile = nil) #:nodoc: - if fields.detect { |f| ! f.kind_of? DirectoryInfo::Field } - raise ArgumentError, "fields must be an array of DirectoryInfo::Field objects" + @valid = true + @fields = [] + + fields.each do |f| + raise ArgumentError, "fields must be an array of DirectoryInfo::Field objects" unless f.kind_of? DirectoryInfo::Field + if f.valid? + @fields << f + else + @valid = false + end end @string = nil # this is used as a flag to indicate that recoding will be necessary - @fields = fields check_begin_end(profile) if profile end + def valid? + @valid + end + # Decode +card+ into a DirectoryInfo object. # # +card+ may either be a something that is convertible to a string using diff --git a/lib/vcard/field.rb b/lib/vcard/field.rb index 37fb96b..1d12e6b 100644 --- a/lib/vcard/field.rb +++ b/lib/vcard/field.rb @@ -101,10 +101,12 @@ def Field.value_str(value) # :nodoc: line end + # Decode a field. def Field.decode0(atline) # :nodoc: if !(atline =~ Bnf::LINE) - raise ::Vcard::InvalidEncodingError, atline + raise(::Vcard::InvalidEncodingError, atline) if ::Vcard.configuration.raise_on_invalid_line? + return false end atgroup = $1.upcase @@ -165,19 +167,27 @@ def Field.decode0(atline) # :nodoc: end end - [ atgroup, atname, atparams, atvalue ] + [ true, atgroup, atname, atparams, atvalue ] end def initialize(line) # :nodoc: @line = line.to_str - @group, @name, @params, @value = Field.decode0(@line) + @valid, @group, @name, @params, @value = Field.decode0(@line) - @params.each do |pname,pvalues| - pvalues.freeze + if valid? + @params.each do |pname,pvalues| + pvalues.freeze + end + else + @group = @name = '' end self end + def valid? + @valid + end + # Create a field by decoding +line+, a String which must already be # unfolded. Decoded fields are frozen, but see #copy(). def Field.decode(line) @@ -323,7 +333,7 @@ def value # Is the #name of this Field +name+? Names are case insensitive. def name?(name) - @name.casecmp(name) == 0 + @name.to_s.casecmp(name) == 0 end # Is the #group of this field +group+? Group names are case insensitive. @@ -590,7 +600,7 @@ def pvalue_idel(pname, pvalue) # new fields, not old fields. def mutate(g, n, p, v) #:nodoc: line = Field.encode0(g, n, p, v) - @group, @name, @params, @value = Field.decode0(line) + @valid, @group, @name, @params, @value = Field.decode0(line) @line = line self rescue ::Vcard::InvalidEncodingError => e diff --git a/lib/vcard/vcard.rb b/lib/vcard/vcard.rb index 5e17d47..332cc29 100644 --- a/lib/vcard/vcard.rb +++ b/lib/vcard/vcard.rb @@ -659,7 +659,8 @@ def self.decode(card) vcards = [] for e in entities - vcards.push(new(e.flatten, "VCARD")) + vcard = new(e.flatten, "VCARD") + vcards.push(vcard) if vcard.valid? || !::Vcard.configuration.ignore_invalid_vcards? end vcards diff --git a/test/field_test.rb b/test/field_test.rb index e779ced..cf5aa2b 100644 --- a/test/field_test.rb +++ b/test/field_test.rb @@ -21,31 +21,31 @@ def test_encode_decode_text() def test_field4 line = "t;e=a,b: 4 " part = Field.decode0(line) - assert_equal("4", part[ 3 ]) + assert_equal("4", part[ 4 ]) end def test_field3 line = "t;e=a,b:4" part = Field.decode0(line) - assert_equal("4", part[ 3 ]) - assert_equal( {"E" => [ "a","b" ] }, part[ 2 ]) + assert_equal("4", part[ 4 ]) + assert_equal( {"E" => [ "a","b" ] }, part[ 3 ]) end def test_field2 line = "tel;type=work,voice,msg:+1 313 747-4454" part = Field.decode0(line) - assert_equal("+1 313 747-4454", part[ 3 ]) - assert_equal( {"TYPE" => [ "work","voice","msg" ] }, part[ 2 ]) + assert_equal("+1 313 747-4454", part[ 4 ]) + assert_equal( {"TYPE" => [ "work","voice","msg" ] }, part[ 3 ]) end def test_field1 line = 'ORGANIZER;CN="xxxx, xxxx [SC100:370:EXCH]":MAILTO:xxxx@americasm01.nt.com' parts = Field.decode0(line) - assert_equal(nil, parts[0]) - assert_equal("ORGANIZER", parts[1]) - assert_equal({ "CN" => [ "xxxx, xxxx [SC100:370:EXCH]" ] }, parts[2]) - assert_equal("MAILTO:xxxx@americasm01.nt.com", parts[3]) + assert_equal(nil, parts[1]) + assert_equal("ORGANIZER", parts[2]) + assert_equal({ "CN" => [ "xxxx, xxxx [SC100:370:EXCH]" ] }, parts[3]) + assert_equal("MAILTO:xxxx@americasm01.nt.com", parts[4]) end =begin this can not be done :-( @@ -67,20 +67,21 @@ def test_case_equiv def test_field0 assert_equal("name:", line = Field.encode0(nil, "name")) - assert_equal([ nil, "NAME", {}, ""], Field.decode0(line)) + assert_equal([ true, nil, "NAME", {}, ""], Field.decode0(line)) assert_equal("name:value", line = Field.encode0(nil, "name", {}, "value")) - assert_equal([ nil, "NAME", {}, "value"], Field.decode0(line)) + assert_equal([ true, nil, "NAME", {}, "value"], Field.decode0(line)) assert_equal("name;encoding=B:dmFsdWU=", line = Field.encode0(nil, "name", { "encoding"=>:b64 }, "value")) - assert_equal([ nil, "NAME", { "ENCODING"=>["B"]}, ["value"].pack("m").chomp ], Field.decode0(line)) + assert_equal([ true, nil, "NAME", { "ENCODING"=>["B"]}, ["value"].pack("m").chomp ], Field.decode0(line)) line = Field.encode0("group", "name", {}, "value") assert_equal "group.name:value", line - assert_equal [ "GROUP", "NAME", {}, "value"], Field.decode0(line) + assert_equal [ true, "GROUP", "NAME", {}, "value"], Field.decode0(line) end - def test_invalid_fields + def test_invalid_fields_wih_raise_error + Vcard::configuration.raise_on_invalid_line = true [ "g.:", ":v", @@ -89,6 +90,16 @@ def test_invalid_fields end end + def test_invalid_fields_wihout_raise_error + Vcard.configuration.raise_on_invalid_line = false + [ + "g.:", + ":v", + ].each do |line| + assert_nothing_raised { Field.decode0(line) } + end + end + def test_date_encode assert_equal("DTSTART:20040101\n", Field.create("DTSTART", Date.new(2004, 1, 1) ).to_s) assert_equal("DTSTART:20040101\n", Field.create("DTSTART", [Date.new(2004, 1, 1)]).to_s) @@ -97,6 +108,7 @@ def test_date_encode def test_field_modify f = Field.create("name") + assert_equal("", f.value) f.value = "" assert_equal("", f.value) diff --git a/test/vcard_test.rb b/test/vcard_test.rb index e2781ff..a922ae3 100644 --- a/test/vcard_test.rb +++ b/test/vcard_test.rb @@ -169,12 +169,50 @@ def _test_cons # FIXME end def test_bad - # FIXME: this should THROW, it's badly encoded! + Vcard::configuration.raise_on_invalid_line = true assert_raises(::Vcard::InvalidEncodingError) do Vcard::Vcard.decode("BEGIN:VCARD\nVERSION:3.0\nKEYencoding=b:this could be \nmy certificate\n\nEND:VCARD\n") end end + def test_not_raise_error_if_configured_to_ignore + Vcard::configuration.raise_on_invalid_line = false + Vcard::configuration.ignore_invalid_vcards = false + assert_nothing_raised do + Vcard::Vcard.decode("BEGIN:VCARD\nVERSION:3.0\nKEYencoding=b:this could be \nmy certificate\n\nEND:VCARD\n") + end + end + + def test_ignore_vcards_with_invalid_fields + Vcard::configuration.raise_on_invalid_line = false + Vcard::configuration.ignore_invalid_vcards = true + src = <<'EOF' +BEGIN:VCARD +VERSION:3.0 +KEYencoding=b:this could be +my certificate +EMAIL:valid@field.value +END:VCARD +BEGIN:VCARD +VERSION:3.0 +EMAIL:valid@field.value +END:VCARD +EOF + + cards = Vcard::Vcard.decode(src) + assert_equal 1, cards.size + end + + def test_ignore_only_invalid_fields + Vcard::configuration.raise_on_invalid_line = false + Vcard::configuration.ignore_invalid_vcards = false + email = 'test@example.com' + cards = Vcard::Vcard.decode("BEGIN:VCARD\nVERSION:3.0\nKEYencoding=b:this could be \nmy certificate\nEMAIL:#{email}\n\nEND:VCARD\n") + assert_equal email, cards.first.email + # [BEGIN, VERSION, EMAIL, END].size == 4 + assert_equal 4, cards.first.fields.size + end + def test_create card = Vcard::Vcard.create key = Vcard::DirectoryInfo.decode("key;type=x509;encoding=B:dGhpcyBjb3VsZCBiZSAKbXkgY2VydGlmaWNhdGUK\n")['key'] From d683f82ec62e2bc02066563be27671a1ca08a606 Mon Sep 17 00:00:00 2001 From: Brendon Muir Date: Tue, 22 Nov 2016 10:18:01 +1300 Subject: [PATCH 2/3] Update README.md --- README.md | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index 00d958c..764032c 100644 --- a/README.md +++ b/README.md @@ -18,22 +18,27 @@ Or install it yourself as: ## Configuration -You can configure how to make deal with invalid lines. -Gem supports three behaviors: +You can configure how to deal with invalid lines. The gem supports three behaviours: + 1. `raise_on_invalid_line = true` - Vcard::InvalidEncodingError will be races if any invalid line will be found + + Vcard::InvalidEncodingError will be raised if any invalid line is found. 2. `raise_on_invalid_line = false, ignore_invalid_vcards = true` - If vcard source has invalid line, then this vcard object will be ignored. - In case if you have only one vcard object in your source string, empty array will be returned from `Vcard.decode` + + If the vcard source has an invalid line, this vcard object will be ignored. + If you have only one vcard object in your source string, an empty array will be returned from `Vcard.decode`. 3. `raise_on_invalid_line = false, ignore_invalid_vcards = false` - vcard will be marked as invalid, invalid field will be ignored, but vcard will be present in `Vcard#decode` results - Vcard.configure do |config| - config.raise_on_invalid_line = false # default true - config.ignore_invalid_vcards = false # default true - end + If the vcard is marked as invalid, invalid fields will be ignored, but the vcard will be present in the results of `Vcard#decode`. + +``` +Vcard.configure do |config| + config.raise_on_invalid_line = false # default true + config.ignore_invalid_vcards = false # default true +end +``` ## Upgrade Notes From cde74eb30564c70afcb313a17444317812ee0676 Mon Sep 17 00:00:00 2001 From: Anton Petrunich Date: Mon, 21 Nov 2016 23:52:59 +0200 Subject: [PATCH 3/3] code cleanup --- lib/vcard/configuration.rb | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/lib/vcard/configuration.rb b/lib/vcard/configuration.rb index 99556d3..42246fc 100644 --- a/lib/vcard/configuration.rb +++ b/lib/vcard/configuration.rb @@ -12,8 +12,5 @@ def initialize @ignore_invalid_vcard = true end - private - - end -end \ No newline at end of file +end