Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow to ignore invalid fields #28

Merged
merged 3 commits into from
Nov 22, 2016
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,30 @@ Or install it yourself as:

$ gem install vcard

## Configuration

You can configure how to deal with invalid lines. The gem supports three behaviours:

1. `raise_on_invalid_line = true`

Vcard::InvalidEncodingError will be raised if any invalid line is found.

2. `raise_on_invalid_line = false, ignore_invalid_vcards = true`

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`

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

We are no longer testing against Ruby 1.8.7.
10 changes: 10 additions & 0 deletions lib/vcard.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
require "open-uri"
require "stringio"

require "vcard/configuration"
require "vcard/attachment"
require "vcard/bnf"
require "vcard/dirinfo"
Expand Down Expand Up @@ -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
19 changes: 19 additions & 0 deletions lib/vcard/configuration.rb
Original file line number Diff line number Diff line change
@@ -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
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you remove this given there are no private methods?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done



end
end
17 changes: 14 additions & 3 deletions lib/vcard/dirinfo.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should there be a condition on this line regarding whether or not to raise?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't change behavior in this place. It looks for me like a strict type check and it's not clear for me why it needed here.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No worries :)

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
Expand Down
24 changes: 17 additions & 7 deletions lib/vcard/field.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion lib/vcard/vcard.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
40 changes: 26 additions & 14 deletions test/field_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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:[email protected]'
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:[email protected]", 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:[email protected]", parts[4])
end

=begin this can not be done :-(
Expand All @@ -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",
Expand All @@ -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)
Expand All @@ -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)
Expand Down
40 changes: 39 additions & 1 deletion test/vcard_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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:[email protected]
END:VCARD
BEGIN:VCARD
VERSION:3.0
EMAIL:[email protected]
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 = '[email protected]'
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']
Expand Down