vcard: nothing above the parser handles a parse error
A PUT now accepts a card whose lines do not all read, where it used to answer valid-address-data: RFC 6352 section 6.3.2.2 has the server keep what it does not understand, and the stored bytes are what goes back out. Assisted-by: Claude Opus 5 via Claude Code
diff --git a/AGENTS.md b/AGENTS.md
index 465e979..5f1a7e3 100644
--- a/AGENTS.md
+++ b/AGENTS.md
@@ -38,7 +38,7 @@ lib/pro_tacts/
├── contact.rb # the one model of a contact: id, vCard, etag,
│ # and the structured accessors over the card
├── vcard.rb # vCard 3.0 escaping and folding, and what a card is
-├── vcard/parser.rb # One card's bytes into properties
+├── vcard/parser.rb # One card's bytes into lines
├── config.rb # Every environment read in the app
├── profile.rb # carddav.mobileconfig generation
└── debug_logger.rb # Full request/response dumps, off by default
@@ -166,12 +166,22 @@ not a fixture; edit a `.vcf` to change what the replay serves.
The etag in `changes` is the exception and is not the same fact — it
is what the card hashed to at that write, which nothing can recompute
once the card moves on.
+- Nothing above `vcard/parser.rb` handles a parse error, and nothing
+ should start. `Parser.lines` is the only read: a line that will not
+ read comes back as a Line with no property. The card is served from
+ its bytes regardless, `VCard` and `Contact` answer from the lines
+ that read, and the index holds what this server understood. There is
+ no repair to make, which is why no caller is asked to look for one.
+- The one exception is `Web#report_broken_assumptions`, and it is not
+ handling: a broken assumption is news that something this server was
+ built on is wrong. It lives at the PUT because that is the arrival —
+ a read happens on every page load, and one bad card must not alert
+ once per page view.
- `vcard.rb` no longer raises on what it does not recognize, and must
not start again. It used to render a hand-edited format, where an
unknown key meant a typo silently losing data; it now reads stored
cards, where an unknown property means a client using the spec, and
- RFC 6352 section 6.3.2.2 requires keeping it. A card that will not
- parse is still served, it just does not get indexed.
+ RFC 6352 section 6.3.2.2 requires keeping it.
- `escape` and `fold` in `vcard.rb` have no caller yet. They are the
writer's half of the module, kept deliberately for PUT and the web
editor, and tested directly rather than through anything that serves.
diff --git a/lib/pro_tacts/contact.rb b/lib/pro_tacts/contact.rb
index 2d64d98..1507001 100644
--- a/lib/pro_tacts/contact.rb
+++ b/lib/pro_tacts/contact.rb
@@ -35,7 +35,7 @@ module ProTacts
# @rbs @id: String
# @rbs @vcard: String
# @rbs @etag: String
- # @rbs @properties: Array[VCard::Property]
+ # @rbs @properties: Array[VCard::Parser::Property]
# Ids end up in paths and arrive from client-supplied hrefs, so an id
# outside this charset cannot be served.
@@ -94,18 +94,16 @@ module ProTacts
# The card's properties, parsed once and memoized — the substrate
# the typed accessors sit on, and the read for what none of them
- # models. Lets VCard::ParseError raise rather than returning an
- # empty card: PUT checks the card's envelope before the store
- # accepts it, so one reaching this unparseable is a bug or a
- # corrupt row, and Sentry (already wired into every request, see
- # ProTacts::Web) is where that should surface, not a silently
- # blank screen. VCard.new tolerates the same bytes for the paths
- # that serve them; this model of a served contact does not.
- #: () -> Array[VCard::Property]
+ # models. Every line that read, and no complaint about one that did
+ # not: a contact is served from its bytes, so a line this parser
+ # cannot read costs an accessor its answer and costs the contact
+ # nothing. There is no repair to make and nobody to make it, which
+ # is why nothing here looks for one.
+ #: () -> Array[VCard::Parser::Property]
def properties
return @properties if defined?(@properties)
- @properties = VCard::Parser.parse(@vcard)
+ @properties = VCard.new(@vcard).properties
end
# FN's value (RFC 2426 section 3.1.1), in text form.
@@ -161,12 +159,12 @@ module ProTacts
private
- #: (String name) -> Array[VCard::Property]
+ #: (String name) -> Array[VCard::Parser::Property]
def of_name(name)
properties.select { it.name.casecmp?(name) }
end
- #: (VCard::Property property) -> Address
+ #: (VCard::Parser::Property property) -> Address
def address_of(property)
po_box, extended, street, locality, region, postal_code, country =
VCard.split_components(property.value)
@@ -176,7 +174,7 @@ module ProTacts
# RFC 2426 section 3.3.1: TYPE can repeat (TYPE=work;TYPE=voice) or
# comma-list (TYPE=work,voice) — either arrives here as one
# parameter per value, so the first is the one this carries.
- #: (VCard::Property property) -> String?
+ #: (VCard::Parser::Property property) -> String?
def type_of(property)
property.parameters.find { |name, _| name.casecmp?("TYPE") }&.last&.downcase
end
diff --git a/lib/pro_tacts/store.rb b/lib/pro_tacts/store.rb
index 3903fbc..4ed95d1 100644
--- a/lib/pro_tacts/store.rb
+++ b/lib/pro_tacts/store.rb
@@ -170,10 +170,10 @@ module ProTacts
# The id of the card whose UID property holds this value, if one
# does — the read behind the no-uid-conflict precondition (RFC 6352
- # section 6.3.2.1). It reads the index, so a card that failed to
- # parse is invisible here; no card that arrived by PUT can be in
- # that state, because PUT rejects what will not parse before
- # storing it. `sole` for the same reason `contact` uses it: two
+ # section 6.3.2.1). It reads the index, so a card whose UID line
+ # would not read is invisible here; no card that arrived by PUT can
+ # be in that state, because PUT reads the UID off the same lines
+ # before storing it. `sole` for the same reason `contact` uses it: two
# cards sharing a UID is a corruption to raise on, not a choice.
#: (String uid) -> String?
def card_id_with_uid(uid)
@@ -220,7 +220,6 @@ module ProTacts
# nothing a client ever saw, and what nobody recognizes is
# reported rather than lost in silence.
card = VCard.new(vcard)
- report_broken_assumptions(card)
existing = birthday_of(id)
birthday, stored =
case card.extract("BDAY")
@@ -281,19 +280,14 @@ module ProTacts
# Drops the index and derives it again from the stored cards alone.
# Always safe to run: nothing is authoritative here, so if the
- # projection ever disagrees with the cards, this is the repair.
- # Returns the ids of cards that did not parse, which are served in
- # full but contribute nothing to the index. Raw rows rather than
- # #contacts, which compose birthdays in: the index reflects what is
- # stored, and no stored card carries a BDAY.
- #: () -> Array[String]
+ # projection ever disagrees with the cards, this is the repair. Raw
+ # rows rather than #contacts, which compose birthdays in: the index
+ # reflects what is stored, and no stored card carries a BDAY.
+ #: () -> void
def rebuild_index
@database.transaction do
card_properties.delete
- cards.order(:id).all.each.with_object([]) do |row, unindexed|
- id = row.fetch(:id).to_s
- unindexed << id unless reindex(id, row.fetch(:vcard).to_s)
- end
+ cards.order(:id).all.each { reindex(it.fetch(:id).to_s, it.fetch(:vcard).to_s) }
end
end
@@ -331,19 +325,19 @@ module ProTacts
# Replaces a card's rows in the index. Rebuilt wholesale rather than
# diffed because it is derived data and replacing it is the cheaper
- # correct thing. Returns whether the card parsed: one that does not
- # is still served, so refusing to index it must not fail the write.
- # Takes the stored card — with the birthday already subtracted — so
- # the index never sees a BDAY no stored card carries.
- #: (String id, String vcard) -> bool
+ # correct thing. Indexes the lines that read and says nothing about
+ # the ones that did not: the card is served from its bytes either
+ # way, and there is no version of this write that fixes a line the
+ # parser cannot read. Takes the stored card — with the birthday
+ # already subtracted — so the index never sees a BDAY no stored
+ # card carries.
+ #: (String id, String vcard) -> void
def reindex(id, vcard)
# Before the parse, so that a card which has stopped parsing does
# not keep the rows from when it did.
card_properties.where(card_id: id).delete
- properties = properties_of(vcard)
- return false if properties.nil?
- properties.each.with_index do |property, position|
+ properties_of(vcard).each.with_index do |property, position|
card_properties.insert(
card_id: id,
position:,
@@ -355,11 +349,10 @@ module ProTacts
card_parameters.insert(card_id: id, position:, name:, value:)
end
end
- true
end
- # A card's properties, or nil when the bytes are not a vCard.
- #: (String vcard) -> Array[VCard::Property]?
+ # A card's properties: every line of it that read.
+ #: (String vcard) -> Array[VCard::Parser::Property]
def properties_of(vcard)
VCard.new(vcard).properties
end
@@ -386,10 +379,10 @@ module ProTacts
# has one and it names BDAY. Found by name rather than position,
# and safe to act on because the line is the unit a rewrite or a
# move to the model carries and a line holds at most one property
- # (VCard::Line) — so moving its bytes moves this and nothing else.
- # Nil for any other shape, the empty line a failed read leaves
- # included.
- #: (VCard::Line line) -> VCard::Property?
+ # (VCard::Parser::Line) — so moving its bytes moves this and
+ # nothing else. Nil for any other shape, the empty line a failed
+ # read leaves included.
+ #: (VCard::Parser::Line line) -> VCard::Parser::Property?
def bday_of(line)
property = line.property
property if property&.name&.casecmp?("BDAY")
@@ -400,7 +393,7 @@ module ProTacts
# it drops unwitnessed, which no client renders and no whitelist
# recognizes. Lines a client rendered are in neither pile: their
# absence is a deletion the rewrite already honors.
- #: (String? vcard) -> [Array[String], Array[VCard::Line]]
+ #: (String? vcard) -> [Array[String], Array[VCard::Parser::Line]]
def carried_and_lost_bday_lines(vcard)
return [[], []] if vcard.nil?
@@ -422,13 +415,13 @@ module ProTacts
# it. The message carries no card content
# (ProTacts::SentryScrubber's line); the values stay on the
# machine, where the admin view shows them raw.
- #: (Array[VCard::Line] lines) -> void
+ #: (Array[VCard::Parser::Line] lines) -> void
def report_unrecognized_bday_lines(lines)
unrecognized = lines.count { |line|
- # A line that broke a parser assumption is not an unrecognized
- # BDAY, it is a line this server declined to read at all —
- # report_broken_assumptions says so, and more precisely.
- next false if line.broke_assumption?
+ # A line with no property is one this server could not read at
+ # all, and a value it never read is not a value it failed to
+ # recognize. The line's bytes are stored either way.
+ next false if line.property.nil?
property = bday_of(line)
property.nil? || (!Birthday.rendered?(property) && !Birthday.unrendered_value?(property.value))
@@ -444,7 +437,7 @@ module ProTacts
# The loss report, the rewrite's half of the arrival one: a stored
# BDAY no client renders and no whitelist recognizes is about to be
# dropped, and nobody would know.
- #: (Array[VCard::Line] lines) -> void
+ #: (Array[VCard::Parser::Line] lines) -> void
def report_lost_bday_lines(lines)
return if lines.empty?
@@ -454,31 +447,6 @@ module ProTacts
)
end
- # The assumption report, louder than the two above it: a submitted
- # card that breaks one of the parser's assumptions is not bad
- # input, it is news that the assumption is wrong and every
- # simplification resting on it is now suspect. Nothing else would
- # say so — the card is stored and served either way, and only its
- # index entry is lost.
- #
- # Here rather than in the parser, which is where the break is
- # found: a parse happens on every read of a card — the admin index
- # parses every contact on every page load — and one bad card must
- # not alert once per page view. An arrival is the event worth a
- # message. The message carries no card content
- # (ProTacts::SentryScrubber's line); the admin view shows the card
- # raw.
- #: (VCard card) -> void
- def report_broken_assumptions(card)
- broken = card.lines.count { it.broke_assumption? }
- return if broken.zero?
-
- Sentry.capture_message(
- "a submitted card broke #{broken} parser assumption(s) about what macOS Contacts sends",
- level: :error,
- )
- end
-
# The etag comes back out of the card rather than out of a column, so
# a row can never carry one that disagrees with the bytes beside it.
# The card is composed with its birthday first, so both halves
diff --git a/lib/pro_tacts/vcard.rb b/lib/pro_tacts/vcard.rb
index 0a891bd..c1641d7 100644
--- a/lib/pro_tacts/vcard.rb
+++ b/lib/pro_tacts/vcard.rb
@@ -2,14 +2,14 @@ module ProTacts
# A card, read once: the parsed properties beside the exact bytes
# they came from, so a caller asks questions of either without
# re-parsing, and moves a property without a byte around it moving
- # too. #lines walks the card once into Line values — a logical line's
- # property beside the exact bytes that carried it — and is the
- # enumeration the questions that move bytes are asked over;
- # #properties folds the same walk down to structure, and #insert
- # puts lines back in. vCard 3.0 (RFC 2426): escape and fold, the
- # writer's half, are here; Parser owns the reading half, down to
- # the line split. The walk is lazy for the same reason Contact's
- # parse is: the byte-moving paths never read structure.
+ # too. #lines is Parser's walk of the card, memoized — a
+ # Parser::Line per logical line — and is the enumeration the
+ # questions that move bytes are asked over; #properties folds the
+ # same walk down to structure, and #insert puts lines back in. vCard
+ # 3.0 (RFC 2426): escape and fold, the writer's half, are here;
+ # Parser owns the reading half, down to the line split and the
+ # values it comes back as. The walk is lazy for the same reason
+ # Contact's parse is: the byte-moving paths never read structure.
class VCard
# Folded lines must not exceed 75 octets, excluding the line break
# (RFC 2426 section 2.6). The octet count, not character count, is
@@ -28,67 +28,6 @@ module ProTacts
# lands inside the envelope however bare the card is.
END_LINE = /\AEND:VCARD/i #: Regexp
- # A card whose bytes are not a vCard at all. Callers are expected to
- # carry on serving the card: failing to parse costs an index entry,
- # not the contact.
- class ParseError < StandardError; end
-
- # One parsed content line.
- #
- # `parameters` is a list of name-and-value pairs rather than a hash
- # because a parameter can repeat, and RFC 2426 section 3.3.1 makes
- # `TYPE=work;TYPE=voice` and `TYPE=work,voice` two spellings of the
- # same thing — both arrive here as two pairs.
- #
- # `value` is the value's text, unfolded and otherwise exactly as it
- # was stored, still escaped. Unescaping it and splitting it on the
- # component separator are per-property semantics, and this layer has
- # none.
- #
- # The signature lives in sig/pro_tacts/vcard.rbs: a Data class has no
- # constant super class for the inline syntax to read.
- # @rbs skip
- Property = Data.define(:group, :name, :parameters, :value)
-
- # One logical line of a card, parsed beside the exact bytes that
- # carried it: `property` is the content line it read, nil when it
- # read none (a blank line, or one that would not read), `error` the
- # ParseError the failure raised, nil when it read, and `verbatim`
- # the line itself, folds and terminator included, so what moves
- # between cards moves unchanged.
- #
- # One property, not a list of them: a logical line can only hold a
- # second content line when a bare CR ends the first, and Parser
- # refuses that outright (Parser::BrokenAssumption). That is what
- # lets a caller move a line's bytes knowing it moves exactly one
- # property.
- #
- # The signature lives in sig/pro_tacts/vcard.rbs with Property's.
- # @rbs skip
- Line = Data.define(:property, :verbatim, :error)
-
- # Reopened rather than defined in the block above: steep reads a
- # define block's self as this class's, not the constant's, so the
- # method could not live there.
- class Line
- # Whether this line names the property, a group prefix
- # (`item1.BDAY`) counting as naming it. Asked of the line rather
- # than the property because a line that will not parse still has
- # a name in it.
- #: (String name) -> bool
- def names?(name)
- verbatim.match?(/\A(?:[A-Za-z0-9-]+\.)?#{Regexp.escape(name)}[;:]/i)
- end
-
- # Whether this line broke one of the parser's assumptions rather
- # than merely failing to read. Asked here so no caller has to
- # know the error taxonomy to tell the two apart.
- #: () -> bool
- def broke_assumption?
- error.is_a?(Parser::BrokenAssumption)
- end
- end
-
# Text values escape backslash, the component separator, and the
# sub-component separator (RFC 2426 section 2.4.2); CRLF and CR are
# normalized to the `\n` escape because a raw line break would end
@@ -151,14 +90,16 @@ module ProTacts
end
# The card's properties, folded off the same single walk #lines is
- # and parsed once on the first structured read. Nil when any line
- # would not read — a caller is expected to carry on serving the
- # bytes (ParseError's rule, held one level up).
- #: () -> Array[Property]?
+ # and parsed once on the first structured read: every line that
+ # read, in order. A line that would not read is not this class's
+ # problem — the bytes are served either way, and a caller that
+ # cannot act on a partial reading asks #lines whether any line was
+ # unreadable before asking anything here.
+ #: () -> Array[Parser::Property]
def properties
return @properties if defined?(@properties)
- @properties = lines.any? { it.error } ? nil : lines.filter_map { it.property }
+ @properties = lines.filter_map { it.property }
end
# Whether the parsed properties carry the envelope RFC 2426 section
@@ -170,13 +111,12 @@ module ProTacts
# and this only decides whether there is a card at all.
#: () -> bool
def card?
- parsed = properties
- return false if parsed.nil? || parsed.empty?
+ first, last = properties.first, properties.last
+ return false if first.nil? || last.nil?
- first, last = parsed.first, parsed.last
first.name.casecmp?("BEGIN") && first.value.casecmp?("VCARD") &&
last.name.casecmp?("END") && last.value.casecmp?("VCARD") &&
- parsed.any? { it.name.casecmp?("VERSION") }
+ properties.any? { it.name.casecmp?("VERSION") }
end
# The value of the card's UID property, if it carries one. Names
@@ -184,8 +124,7 @@ module ProTacts
# assumes for them.
#: () -> String?
def uid
- parsed = properties
- parsed && parsed.find { it.name.casecmp?("UID") }&.value
+ properties.find { it.name.casecmp?("UID") }&.value
end
# The lines naming `name`, and the card without them: the split
@@ -193,7 +132,7 @@ module ProTacts
# rejoin the rest by hand and none can lose a byte doing it. The
# taken lines come back parsed beside their bytes; what is left
# comes back a card, ready to store or to take another split.
- #: (String name) -> [Array[Line], VCard]
+ #: (String name) -> [Array[Parser::Line], VCard]
def extract(name)
taken, rest = lines.partition { it.names?(name) }
[taken, VCard.new(rest.map(&:verbatim).join)]
@@ -208,7 +147,7 @@ module ProTacts
# continuation travels with its line, terminators attached, so no
# line's bytes are lost or normalized on the way through (RFC 2426
# section 2.6 folding).
- #: () -> Array[Line]
+ #: () -> Array[Parser::Line]
def lines
return @lines if defined?(@lines)
diff --git a/lib/pro_tacts/vcard/parser.rb b/lib/pro_tacts/vcard/parser.rb
index caca4f6..e1729d8 100644
--- a/lib/pro_tacts/vcard/parser.rb
+++ b/lib/pro_tacts/vcard/parser.rb
@@ -5,7 +5,7 @@ require "pro_tacts/vcard"
module ProTacts
class VCard
- # Reads a stored card into VCard::Property and VCard::Line values.
+ # Reads a card's bytes into Lines.
#
# Deliberately shallow: a property is a name, its parameters, and the
# text of its value, and nothing here knows what any of them mean.
@@ -15,18 +15,29 @@ module ProTacts
# 6.3.2.2 requires of a CardDAV server. See
# docs/plans/2026-08-24-vcard-storage-and-groups.md.
#
- # The whole reading half of VCard lives here: unfolding, the split
- # into logical lines, and the reads over them. One instance reads one
- # logical line — the scanner is the whole of its state, which is why
- # it is a class and not a handful of functions passing a position
- # between them — and the two class-level reads differ only in
- # strictness. parse raises on a line that will not read, which is
- # what keeps an unparseable card served but unindexed; lines carries
- # the same line with a nil property, which is what lets a rewrite
- # move bytes no line could ever parse out of.
+ # The whole reading half of VCard lives here — unfolding, the split
+ # into logical lines, the reads over them, and the values they come
+ # back as — and it has one entry point: lines. One instance reads
+ # one logical line; the scanner is the whole of its state, which is
+ # why it is a class and not a handful of functions passing a
+ # position between them.
+ #
+ # A line that will not read is a Line like any other, carrying the
+ # error rather than raising it, and no read here raises on a card's
+ # behalf. Nothing above handles one either: VCard and Contact answer
+ # from the lines that read, the index holds what this server
+ # understood, and a rewrite moves the bytes regardless. The error
+ # rides along for one reader, ProTacts::Web, which reports a broken
+ # assumption at the arrival that carried it.
class Parser
# @rbs @scanner: StringScanner
+ # A line whose bytes are not a content line at all. Never raised
+ # past #lines, which catches it and hands it back on the Line it
+ # happened to: what a line failing to read costs is that line's
+ # index rows, and the card is served from its bytes regardless.
+ class ParseError < StandardError; end
+
# Something this parser was built to assume it would never read.
# Not a judgment on the card: the assumptions are about macOS
# Contacts, the only client this address book serves
@@ -34,12 +45,67 @@ module ProTacts
# card can break one while being a perfectly good vCard. Raising
# is how the assumption gets checked instead of merely held.
#
- # A ParseError so it rides the same rails as any card that will
- # not read — the bytes are still served, an index entry is what
- # is lost — and its own class so Store can report it, a wrong
- # assumption being news rather than ordinary bad input.
+ # A ParseError so it rides the same rails as any line that will
+ # not read — the bytes are still served, index rows are what is
+ # lost — and its own class so ProTacts::Web can report it at the
+ # arrival, a wrong assumption being news rather than ordinary bad
+ # input.
class BrokenAssumption < ParseError; end
+ # One parsed content line.
+ #
+ # `parameters` is a list of name-and-value pairs rather than a hash
+ # because a parameter can repeat, and RFC 2426 section 3.3.1 makes
+ # `TYPE=work;TYPE=voice` and `TYPE=work,voice` two spellings of the
+ # same thing — both arrive here as two pairs.
+ #
+ # `value` is the value's text, unfolded and otherwise exactly as it
+ # was stored, still escaped. Unescaping it and splitting it on the
+ # component separator are per-property semantics, and this layer has
+ # none.
+ #
+ # The signature lives in sig/pro_tacts/vcard/parser.rbs: a Data
+ # class has no constant super class for the inline syntax to read.
+ # @rbs skip
+ Property = Data.define(:group, :name, :parameters, :value)
+
+ # One logical line of a card, parsed beside the exact bytes that
+ # carried it: `property` is the content line it read, nil when it
+ # read none (a blank line, or one that would not read), `error` the
+ # ParseError the failure raised, nil when it read, and `verbatim`
+ # the line itself, folds and terminator included, so what moves
+ # between cards moves unchanged.
+ #
+ # One property, not a list of them: a logical line can only hold a
+ # second content line when a bare CR ends the first, and that is
+ # refused outright (BrokenAssumption). That is what lets a caller
+ # move a line's bytes knowing it moves exactly one property.
+ #
+ # The signature lives in sig/pro_tacts/vcard/parser.rbs with
+ # Property's.
+ # @rbs skip
+ Line = Data.define(:property, :verbatim, :error)
+
+ # Reopened rather than defined in the block above: steep reads a
+ # define block's self as this class's, not the constant's, so the
+ # methods could not live there.
+ class Line
+ # Whether this line names the property, a group prefix
+ # (`item1.BDAY`) counting as naming it. Asked of the line rather
+ # than the property because a line that will not parse still has
+ # a name in it.
+ #: (String name) -> bool
+ def names?(name)
+ verbatim.match?(/\A(?:[A-Za-z0-9-]+\.)?#{Regexp.escape(name)}[;:]/i)
+ end
+
+ # Whether this line broke one of the parser's assumptions rather
+ # than merely failing to read. Asked here so no caller has to
+ # know the error taxonomy to tell the two apart.
+ #: () -> bool
+ def broke_assumption? = error.is_a?(BrokenAssumption)
+ end
+
# The content line, RFC 2426 section 4:
#
# contentline = [group "."] name *(";" param) ":" value CRLF
@@ -72,25 +138,13 @@ module ProTacts
# 2.6 folding).
CONTINUATION = /\A[ \t]/ #: Regexp
- # Every content line in the card, in order. Blank lines are
- # skipped, and BEGIN, VERSION, and END come back like any other
- # property: deciding that a card is well-formed is not this class's
- # job. Raises the error of the first line that will not read, so a
- # caller rescuing ParseError declines the whole card at once.
- #: (String card) -> Array[Property]
- def self.parse(card)
- lines = lines(card)
- error = lines.filter_map { it.error }.first
- raise error if error
-
- lines.filter_map { it.property }
- end
-
# The card's logical lines, each parsed beside the exact bytes it
# came from, folds and terminator included — the one walk every
- # read of a card derives from. A line that will not read is a
- # fact about the line, not an error: the error it raised rides on
- # it, and the bytes stay enumerable.
+ # read of a card derives from. Blank lines are lines, and BEGIN,
+ # VERSION, and END read like any other property: deciding that a
+ # card is well-formed is not this class's job. A line that will
+ # not read is a fact about the line, not an error: the error it
+ # raised rides on it, and the bytes stay enumerable.
#: (String card) -> Array[Line]
def self.lines(card)
logical_lines(card).map { |logical_line| line_of(logical_line) }
diff --git a/lib/pro_tacts/web.rb b/lib/pro_tacts/web.rb
index 71d25a7..f5ed746 100644
--- a/lib/pro_tacts/web.rb
+++ b/lib/pro_tacts/web.rb
@@ -442,10 +442,16 @@ module ProTacts
return precondition("supported-address-data") unless request.media_type == "text/vcard"
# CARDDAV:valid-address-data: bytes that are the UTF-8 the media
- # type declares, parsing into a card with the envelope RFC 2426
- # section 4 requires. Invalid UTF-8 would otherwise surface as a
- # 500 out of SQLite on the insert, past the point where the
- # client could be told what was wrong with the body.
+ # type declares, carrying the envelope RFC 2426 section 4
+ # requires. Invalid UTF-8 would otherwise surface as a 500 out of
+ # SQLite on the insert, past the point where the client could be
+ # told what was wrong with the body.
+ #
+ # The envelope is the whole test. A line inside it this parser
+ # cannot read is not grounds for refusing the card: RFC 6352
+ # section 6.3.2.2 has the server keep what it does not
+ # understand, and it is kept — the stored bytes are what goes
+ # back out.
return precondition("valid-address-data") unless vcard.valid_encoding?
card = VCard.new(vcard)
@@ -473,6 +479,8 @@ module ProTacts
if_none_match = request.env["HTTP_IF_NONE_MATCH"]
return plain_412 if if_none_match && if_none_match_failed?(if_none_match, existing)
+ report_broken_assumptions(card)
+
stored = store.put(id, vcard)
response.status = existing ? 204 : 201
# A strong ETag belongs on the answer only when what was stored is
@@ -491,6 +499,30 @@ module ProTacts
nil
end
+ # A card that breaks one of the parser's assumptions is not bad
+ # input, it is news that the assumption is wrong and every
+ # simplification resting on it is now suspect. Nothing else would
+ # say so: the card is accepted, stored, and served either way, and
+ # a line that will not read costs it only its index rows.
+ #
+ # Here rather than in the parser, which is where the break is
+ # found: a read happens on every look at a card — the admin index
+ # reads every contact on every page load — and one bad card must
+ # not alert once per page view. An arrival is the event worth a
+ # message, and a PUT is the only arrival there is. The message
+ # carries no card content (ProTacts::SentryScrubber's line); the
+ # admin view shows the card raw.
+ #: (VCard card) -> void
+ def report_broken_assumptions(card)
+ broken = card.lines.count { it.broke_assumption? }
+ return if broken.zero?
+
+ Sentry.capture_message(
+ "a submitted card broke #{broken} parser assumption(s) about what macOS Contacts sends",
+ level: :error,
+ )
+ end
+
# A 412 whose body names the CardDAV precondition that failed, in
# the DAV:error form RFC 4918 section 16 defines; the element names
# are RFC 6352 section 6.3.2.1's, qualified by the card: namespace
diff --git a/sig/pro_tacts/birthday.rbs b/sig/pro_tacts/birthday.rbs
index aec6546..35dbb42 100644
--- a/sig/pro_tacts/birthday.rbs
+++ b/sig/pro_tacts/birthday.rbs
@@ -19,12 +19,12 @@ module ProTacts
def served?: () -> bool
- def self.from_property: (VCard::Property property) -> Birthday?
+ def self.from_property: (VCard::Parser::Property property) -> Birthday?
- def self.apple_no_year?: (VCard::Property property) -> bool
+ def self.apple_no_year?: (VCard::Parser::Property property) -> bool
def self.unrendered_value?: (String value) -> bool
- def self.rendered?: (VCard::Property property) -> bool
+ def self.rendered?: (VCard::Parser::Property property) -> bool
end
end
diff --git a/sig/pro_tacts/vcard.rbs b/sig/pro_tacts/vcard.rbs
deleted file mode 100644
index c760424..0000000
--- a/sig/pro_tacts/vcard.rbs
+++ /dev/null
@@ -1,23 +0,0 @@
-# Property and Line are Data classes, which the inline syntax cannot
-# read — Data.define leaves them no constant super class — so their
-# signatures live here. VCard itself is signed inline in vcard.rb.
-module ProTacts
- class VCard
- class Property
- attr_reader group: String?
- attr_reader name: String
- attr_reader parameters: Array[[String, String]]
- attr_reader value: String
-
- def self.new: (group: String?, name: String, parameters: Array[[String, String]], value: String) -> instance
- end
-
- class Line
- attr_reader property: Property?
- attr_reader verbatim: String
- attr_reader error: ParseError?
-
- def self.new: (property: Property?, verbatim: String, error: ParseError?) -> instance
- end
- end
-end
diff --git a/sig/pro_tacts/vcard/parser.rbs b/sig/pro_tacts/vcard/parser.rbs
new file mode 100644
index 0000000..c718761
--- /dev/null
+++ b/sig/pro_tacts/vcard/parser.rbs
@@ -0,0 +1,25 @@
+# Property and Line are Data classes, which the inline syntax cannot
+# read — Data.define leaves them no constant super class — so their
+# signatures live here. Parser itself is signed inline in parser.rb.
+module ProTacts
+ class VCard
+ class Parser
+ class Property
+ attr_reader group: String?
+ attr_reader name: String
+ attr_reader parameters: Array[[String, String]]
+ attr_reader value: String
+
+ def self.new: (group: String?, name: String, parameters: Array[[String, String]], value: String) -> instance
+ end
+
+ class Line
+ attr_reader property: Property?
+ attr_reader verbatim: String
+ attr_reader error: ParseError?
+
+ def self.new: (property: Property?, verbatim: String, error: ParseError?) -> instance
+ end
+ end
+ end
+end
diff --git a/tasks/index.rake b/tasks/index.rake
index 1719bf1..f346796 100644
--- a/tasks/index.rake
+++ b/tasks/index.rake
@@ -11,9 +11,8 @@ namespace :index do
database = ProTacts.config.database_path
ProTacts::Store.connect(database) do |store|
- unindexed = store.rebuild_index
+ store.rebuild_index
puts "rebuilt the index in #{database}"
- warn "not indexed, still served: #{unindexed.join(', ')}" unless unindexed.empty?
end
end
end
diff --git a/test/pro_tacts/test_birthday.rb b/test/pro_tacts/test_birthday.rb
index 00433dd..7a1652b 100644
--- a/test/pro_tacts/test_birthday.rb
+++ b/test/pro_tacts/test_birthday.rb
@@ -65,7 +65,7 @@ class BirthdayTest < Minitest::Test
# A parsed property, the shape a read out of a card takes.
def property(value, parameters: [], group: nil)
- ProTacts::VCard::Property.new(group:, name: "BDAY", parameters:, value:)
+ ProTacts::VCard::Parser::Property.new(group:, name: "BDAY", parameters:, value:)
end
def test_a_full_date_reads_into_the_model
diff --git a/test/pro_tacts/test_contact.rb b/test/pro_tacts/test_contact.rb
index 8344257..1dc3031 100644
--- a/test/pro_tacts/test_contact.rb
+++ b/test/pro_tacts/test_contact.rb
@@ -149,12 +149,13 @@ class ContactTest < Minitest::Test
assert_nil bare.notes
end
- # Unlike Store#rebuild_index, which fails a card open because a
- # write already guarantees every stored card parses (VCard.card? is
- # checked before a PUT is accepted) — a card that reaches here and
- # still won't parse is a bug or a corrupt row, not the ordinary case,
- # and it should raise into Sentry rather than render a blank card.
- def test_a_card_that_will_not_parse_raises_from_the_accessors
- assert_raises(ProTacts::VCard::ParseError) { contact("not a vCard at all").phones }
+ # A card that will not read is still a contact: the bytes are what
+ # gets served, and no accessor has a repair to offer for a line the
+ # parser cannot read. The accessors answer from the lines that read
+ # and say nothing about the rest.
+ def test_a_card_that_will_not_parse_answers_from_what_read
+ assert_empty contact("not a vCard at all").phones
+ assert_empty contact("not a vCard at all").properties
+ assert_equal "Aiden", contact("FN:Aiden\r\nTEL;HOME:+1-555-1234\r\n").name
end
end
diff --git a/test/pro_tacts/test_store.rb b/test/pro_tacts/test_store.rb
index f213996..43325e2 100644
--- a/test/pro_tacts/test_store.rb
+++ b/test/pro_tacts/test_store.rb
@@ -8,6 +8,8 @@ require "sequel"
require "pro_tacts/store"
class StoreTest < Minitest::Test
+ include CapturingSentry
+
AIDEN = "BEGIN:VCARD\r\nVERSION:3.0\r\nFN:Aiden\r\nUID:aiden\r\nEND:VCARD\r\n"
# UTC ISO 8601 to the millisecond, which is what SQLite is asked for.
TIMESTAMP = /\A\d{4}-\d\d-\d\dT\d\d:\d\d:\d\d\.\d{3}Z\z/
@@ -15,23 +17,6 @@ class StoreTest < Minitest::Test
# A store on disk rather than in memory: WAL, the busy timeout, and
# reopening a database are all part of what is under test.
- # minitest 6 ships no mock, so the Sentry capture is stubbed by hand:
- # the bound original is swapped back in an ensure, so one assertion
- # cannot leak into the next. Sentry is never initialized under test,
- # so the original is inert anyway — this only makes it observable.
- # Returns what was captured.
- def capturing_sentry
- messages = []
- original = Sentry.method(:capture_message)
- Sentry.define_singleton_method(:capture_message) { |message, **| messages << message }
- begin
- yield messages
- ensure
- Sentry.define_singleton_method(:capture_message, original)
- end
- messages
- end
-
def with_store(cards = {})
Dir.mktmpdir do |dir|
ProTacts::Store.connect(Pathname.new(dir) / "contacts.db") do |store|
@@ -413,7 +398,8 @@ class StoreTest < Minitest::Test
wreck_the_index(store)
refute_equal before, index_rows(store)
- assert_empty store.rebuild_index
+ store.rebuild_index
+
assert_equal before, index_rows(store)
end
end
@@ -437,8 +423,8 @@ class StoreTest < Minitest::Test
## Cards that will not parse
# Fail open: the bytes are what gets served, so a card the parser
- # cannot read is still a contact. It just contributes nothing to the
- # index, and the rebuild names it.
+ # cannot read is still a contact. Its unreadable lines contribute
+ # nothing to the index and cost the index nothing else.
def test_an_unparseable_card_is_still_stored_and_served
with_store({"broken" => "this is not a vCard\r\n"}) do |store|
assert_equal "this is not a vCard\r\n", store.contact("broken").vcard
@@ -446,10 +432,14 @@ class StoreTest < Minitest::Test
end
end
- def test_rebuilding_names_the_cards_it_could_not_index
- with_store({"aiden" => AIDEN, "broken" => "this is not a vCard\r\n"}) do |store|
- assert_equal %w[broken], store.rebuild_index
+ # One line the parser cannot read does not cost the card the lines
+ # that read: the index is what this server understood, not an
+ # all-or-nothing verdict on the card.
+ def test_a_card_is_indexed_by_the_lines_that_read
+ unreadable = AIDEN.sub("FN:Aiden\r\n", "FN:Aiden\r\nTEL;HOME:+1-555-1234\r\n")
+ with_store({"aiden" => unreadable}) do |store|
assert_equal %w[BEGIN VERSION FN UID END], indexed_names(store, "aiden")
+ assert_equal unreadable, store.contact("aiden").vcard
end
end
@@ -551,11 +541,13 @@ class StoreTest < Minitest::Test
# dropped from the card").
# A BDAY sharing its line's bytes with another content line — a bare
# CR packs two into one physical line — is a shape the parser is
- # built to assume macOS never sends, so it is refused rather than
- # read: the line stays verbatim, the model empties, and the broken
- # assumption is reported as itself rather than as an odd BDAY, which
- # is what would send anyone reading it looking in the wrong place.
- def test_a_bday_sharing_its_line_arrives_whole_and_reported
+ # built to assume macOS never sends, so it is never read: the line
+ # stays verbatim and the model empties. The store says nothing about
+ # it. A value it never read is not a value it failed to recognize,
+ # and reporting it as an odd BDAY would send anyone reading the
+ # message looking in the wrong place. WebTest holds the report that
+ # is worth making, at the arrival.
+ def test_a_bday_sharing_its_line_arrives_whole_and_unreported
shared = AIDEN.sub("END:VCARD\r\n", "BDAY:1985-04-12\rNOTE:b\r\nEND:VCARD\r\n")
with_store({"aiden" => AIDEN_BORN}) do |store|
@@ -563,8 +555,7 @@ class StoreTest < Minitest::Test
assert_equal shared, store.contact("aiden").vcard
assert_nil birthday_row(store, "aiden")
- assert_equal 1, messages.length
- assert_match "broke 1 parser assumption", messages.fetch(0)
+ assert_empty messages
end
end
diff --git a/test/pro_tacts/test_web.rb b/test/pro_tacts/test_web.rb
index b942ddd..820ed03 100644
--- a/test/pro_tacts/test_web.rb
+++ b/test/pro_tacts/test_web.rb
@@ -11,6 +11,7 @@ require "pro_tacts/store"
require "pro_tacts/web"
class WebTest < Minitest::Test
+ include CapturingSentry
include Rack::Test::Methods
def app
@@ -382,6 +383,54 @@ class WebTest < Minitest::Test
end
end
+ # A line that will not read is not grounds for refusing the card —
+ # RFC 6352 section 6.3.2.2 has the server keep what it does not
+ # understand — so the PUT succeeds and the bytes go back out whole.
+ def test_put_of_a_card_with_a_line_that_will_not_read_is_accepted_whole
+ unreadable = card("new", "New").sub("FN:New\r\n", "FN:New\r\nTEL;HOME:+1-555-1234\r\n")
+
+ with_contacts({}) do
+ put_request "new", unreadable, "CONTENT_TYPE" => VCARD
+
+ assert_equal 201, last_response.status
+
+ get "/dav/addressbook/new.vcf"
+
+ assert_equal unreadable, last_response.body
+ end
+ end
+
+ # The one report left in the app: a card that breaks an assumption
+ # about what macOS Contacts sends is news that the assumption is
+ # wrong, and an arrival is the event worth a message — a read happens
+ # on every page load. A bare CR packs two content lines into one
+ # line's bytes, which is the assumption this card breaks.
+ def test_put_of_a_card_breaking_a_parser_assumption_is_reported
+ packed = card("new", "New").sub("FN:New\r\n", "FN:New\rNOTE:b\r\n")
+
+ with_contacts({}) do
+ messages = capturing_sentry {
+ put_request "new", packed, "CONTENT_TYPE" => VCARD
+ }
+
+ assert_equal 201, last_response.status
+ assert_equal 1, messages.length
+ assert_match "broke 1 parser assumption", messages.fetch(0)
+ end
+ end
+
+ # Nothing else reports: an ordinary card arrives in silence.
+ def test_put_of_an_ordinary_card_is_quiet
+ with_contacts({}) do
+ messages = capturing_sentry {
+ put_request "new", card("new", "New"), "CONTENT_TYPE" => VCARD
+ }
+
+ assert_equal 201, last_response.status
+ assert_empty messages
+ end
+ end
+
def test_put_of_a_card_whose_uid_belongs_to_another_card
with_contacts({"aiden" => "Aiden"}) do
put_request "znorth", card("aiden", "Aiden"), "CONTENT_TYPE" => VCARD
diff --git a/test/pro_tacts/vcard/test_lines.rb b/test/pro_tacts/vcard/test_lines.rb
index 62078e7..2125757 100644
--- a/test/pro_tacts/vcard/test_lines.rb
+++ b/test/pro_tacts/vcard/test_lines.rb
@@ -15,13 +15,28 @@ class VCardLinesTest < Minitest::Test
assert_equal %w[BEGIN VERSION FN UID END], card.properties.map(&:name)
end
- # The parse is lazy, so moving bytes never pays for reading structure.
- def test_the_parse_happens_only_when_read
+ # A card whose bytes will not read is still a card here: it answers
+ # every question from the lines that did read, and never raises. A
+ # line the parser cannot read is nobody's problem to solve — the
+ # bytes are served either way — so nothing above the parser is asked
+ # to notice one.
+ def test_a_card_that_will_not_read_answers_from_what_did
card = VCARD.new("this is not a vCard\r\n")
- assert_nil card.properties
+ assert_empty card.properties
refute card.card?
assert_nil card.uid
+ assert_equal ["this is not a vCard\r\n"], card.lines.map(&:verbatim)
+ end
+
+ # Only the lines that would not read drop out; the card answers from
+ # the rest, envelope included.
+ def test_a_card_answers_around_a_line_that_will_not_read
+ card = VCARD.new(CARD.sub("FN:Ada\r\n", "FN:Ada\r\nTEL;HOME:+1-555-1234\r\n"))
+
+ assert_equal %w[BEGIN VERSION FN UID END], card.properties.map(&:name)
+ assert card.card?
+ assert_equal "ada", card.uid
end
# A card is made of UTF-8 text and refuses to be made of anything
@@ -103,7 +118,7 @@ class VCardLinesTest < Minitest::Test
unparsed = VCARD.new("BDAY;=;:\r\n").lines.first
- assert_kind_of VCARD::ParseError, unparsed.error
+ assert_kind_of VCARD::Parser::ParseError, unparsed.error
assert_nil unparsed.property
assert unparsed.names?("BDAY")
assert_equal "BDAY;=;:\r\n", unparsed.verbatim
@@ -122,7 +137,7 @@ class VCardLinesTest < Minitest::Test
unreadable = VCARD.new("BEGIN:VCARD\r\nBDAY;=;:\r\nEND:VCARD\r\n")
- assert_kind_of VCARD::ParseError, unreadable.lines.fetch(1).error
+ assert_kind_of VCARD::Parser::ParseError, unreadable.lines.fetch(1).error
refute unreadable.lines.fetch(1).broke_assumption?
refute VCARD.new(CARD).lines.any? { it.broke_assumption? }
diff --git a/test/pro_tacts/vcard/test_parser.rb b/test/pro_tacts/vcard/test_parser.rb
index 1ea1a98..03bcc8f 100644
--- a/test/pro_tacts/vcard/test_parser.rb
+++ b/test/pro_tacts/vcard/test_parser.rb
@@ -7,8 +7,22 @@ require "pro_tacts/vcard/parser"
class VCardParserTest < Minitest::Test
include Hegel::Syntax::Methods
- def parse(card)
- ProTacts::VCard::Parser.parse(card)
+ def lines(card)
+ ProTacts::VCard::Parser.lines(card)
+ end
+
+ # What the card's lines read, which for a card whose every line reads
+ # is the card. The parser hands back lines and only lines, so folding
+ # them down to structure is the caller's move — this is the one every
+ # test below that asks about a property makes.
+ def properties(card)
+ lines(card).filter_map { it.property }
+ end
+
+ # The error the one line of a card carried, for the tests that are
+ # about a line that will not read.
+ def error(card)
+ lines(card).fetch(0).error
end
# A card as it arrives: CRLF line breaks, trailing break included.
@@ -19,61 +33,61 @@ class VCardParserTest < Minitest::Test
## Content lines
def test_every_line_comes_back_as_a_property
- properties = parse(card(
+ parsed = properties(card(
"BEGIN:VCARD",
"VERSION:3.0",
"FN:Test Contact",
"END:VCARD",
))
- assert_equal %w[BEGIN VERSION FN END], properties.map { it.name }
- assert_equal %w[VCARD 3.0], properties.first(2).map { it.value }
+ assert_equal %w[BEGIN VERSION FN END], parsed.map { it.name }
+ assert_equal %w[VCARD 3.0], parsed.first(2).map { it.value }
end
def test_a_value_may_contain_colons_and_spaces
- property = parse(card("URL:https://example.com/a:b")).first
+ property = properties(card("URL:https://example.com/a:b")).first
assert_equal "https://example.com/a:b", property.value
end
def test_a_value_may_be_empty
- property = parse(card("NOTE:")).first
+ property = properties(card("NOTE:")).first
assert_equal "NOTE", property.name
assert_equal "", property.value
end
def test_a_value_keeps_its_escaping
- property = parse(card("FN:Smith\\, John\; Jr.")).first
+ property = properties(card("FN:Smith\\, John\; Jr.")).first
assert_equal "Smith\\, John\; Jr.", property.value
end
def test_a_group_is_split_off_the_name
- properties = parse(card("item1.X-ABLabel:_$!<Home>!$_", "X-ABADR:us"))
+ parsed = properties(card("item1.X-ABLabel:_$!<Home>!$_", "X-ABADR:us"))
- assert_equal ["item1", nil], properties.map { it.group }
- assert_equal %w[X-ABLabel X-ABADR], properties.map { it.name }
+ assert_equal ["item1", nil], parsed.map { it.group }
+ assert_equal %w[X-ABLabel X-ABADR], parsed.map { it.name }
end
def test_blank_lines_are_skipped
- assert_equal %w[FN UID], parse("FN:A\r\n\r\nUID:b\r\n").map { it.name }
+ assert_equal %w[FN UID], properties("FN:A\r\n\r\nUID:b\r\n").map { it.name }
end
def test_a_final_line_break_is_optional
- assert_equal %w[END], parse("END:VCARD").map { it.name }
+ assert_equal %w[END], properties("END:VCARD").map { it.name }
end
# Contacts sends CRLF, but a card that reached the store some other way
# should not be unreadable over a line ending.
def test_bare_line_feeds_are_accepted
- assert_equal %w[FN UID], parse("FN:A\nUID:b\n").map { it.name }
+ assert_equal %w[FN UID], properties("FN:A\nUID:b\n").map { it.name }
end
## Parameters
def test_a_parameter_is_a_name_and_a_value
- property = parse(card("TEL;TYPE=work:+1-555-1234")).first
+ property = properties(card("TEL;TYPE=work:+1-555-1234")).first
assert_equal [%w[TYPE work]], property.parameters
assert_equal "+1-555-1234", property.value
@@ -83,8 +97,8 @@ class VCardParserTest < Minitest::Test
# same pairs. Contacts writes the first (see
# docs/plans/2026-08-24-corrections-from-the-first-write.md).
def test_a_repeated_parameter_and_a_value_list_agree
- repeated = parse(card("ADR;type=HOME;type=pref:;;1 Main St;;;;")).first
- listed = parse(card("ADR;type=HOME,pref:;;1 Main St;;;;")).first
+ repeated = properties(card("ADR;type=HOME;type=pref:;;1 Main St;;;;")).first
+ listed = properties(card("ADR;type=HOME,pref:;;1 Main St;;;;")).first
assert_equal [%w[type HOME], %w[type pref]], repeated.parameters
assert_equal repeated.parameters, listed.parameters
@@ -93,14 +107,14 @@ class VCardParserTest < Minitest::Test
# A quoted value is how a parameter carries the delimiters that would
# otherwise end it.
def test_a_quoted_parameter_value_keeps_its_delimiters
- property = parse(card('PHOTO;X-NOTE="a;b:c,d":data')).first
+ property = properties(card('PHOTO;X-NOTE="a;b:c,d":data')).first
assert_equal [["X-NOTE", "a;b:c,d"]], property.parameters
assert_equal "data", property.value
end
def test_an_empty_parameter_value_is_allowed
- assert_equal [["TYPE", ""]], parse(card("TEL;TYPE=:+1-555-1234")).first.parameters
+ assert_equal [["TYPE", ""]], properties(card("TEL;TYPE=:+1-555-1234")).first.parameters
end
## Folding
@@ -109,7 +123,7 @@ class VCardParserTest < Minitest::Test
# it, and nothing else: a fold falls between two octets, not between
# two words, so a space either side of it is content.
def test_a_folded_line_is_one_property
- property = parse(card("NOTE:supercalifragilistic", " expialidocious")).first
+ property = properties(card("NOTE:supercalifragilistic", " expialidocious")).first
assert_equal "supercalifragilisticexpialidocious", property.value
end
@@ -117,7 +131,7 @@ class VCardParserTest < Minitest::Test
# A fold lands wherever 75 octets falls, which is not always in the
# value: Contacts does not look at what it is splitting.
def test_a_fold_inside_the_parameters_is_unfolded_too
- property = parse(card("TEL;TY", " PE=work:+1-555-1234")).first
+ property = properties(card("TEL;TY", " PE=work:+1-555-1234")).first
assert_equal [%w[TYPE work]], property.parameters
assert_equal "+1-555-1234", property.value
@@ -128,21 +142,21 @@ class VCardParserTest < Minitest::Test
# above rather than standing alone, and whatever whitespace is left
# over becomes part of that value.
def test_a_whitespace_only_line_folds_into_the_one_above_it
- properties = parse(card("FN:A", " ", "UID:b"))
+ parsed = properties(card("FN:A", " ", "UID:b"))
- assert_equal %w[FN UID], properties.map { it.name }
- assert_equal "A ", properties.first.value
+ assert_equal %w[FN UID], parsed.map { it.name }
+ assert_equal "A ", parsed.first.value
end
# With no line above it to fold into, the same whitespace is simply not
# a content line. One stray line at the top of a card therefore costs
# the whole card its index entry — it is still served, from its bytes.
- def test_a_leading_whitespace_only_line_raises
- assert_raises(ProTacts::VCard::ParseError) { parse(card(" ", "FN:A")) }
+ def test_a_leading_whitespace_only_line_does_not_read
+ assert_kind_of ProTacts::VCard::Parser::ParseError, error(card(" ", "FN:A"))
end
def test_a_continuation_may_start_with_a_tab
- assert_equal "abcd", parse(card("NOTE:ab", "\tcd")).first.value
+ assert_equal "abcd", properties(card("NOTE:ab", "\tcd")).first.value
end
## Lines
@@ -170,78 +184,78 @@ class VCardParserTest < Minitest::Test
assert_kind_of ProTacts::VCard::Parser::BrokenAssumption, line.error
assert_nil line.property
assert_equal "FN:A\rNOTE:n\r\n", line.verbatim
-
- assert_raises(ProTacts::VCard::Parser::BrokenAssumption) { parse("FN:A\rNOTE:n\r\n") }
+ assert_empty properties("FN:A\rNOTE:n\r\n")
end
# A bare CR that ends the only content line on its line takes no
# second property with it, so there is nothing to refuse.
def test_a_bare_cr_ending_a_line_alone_still_reads
- assert_equal %w[FN], parse("FN:A\r").map { it.name }
+ assert_equal %w[FN], properties("FN:A\r").map { it.name }
end
- # BrokenAssumption is a ParseError, so a caller that declines a card
- # it cannot read declines this one too without knowing the
- # difference — the class is for reporting, not for control flow.
+ # BrokenAssumption is a ParseError: it reads no differently from a
+ # line that merely will not read, and the class exists so Store can
+ # report the break, not so anything can branch on it.
def test_a_broken_assumption_is_a_parse_error
- assert_operator ProTacts::VCard::Parser::BrokenAssumption, :<, ProTacts::VCard::ParseError
- assert_nil ProTacts::VCard.new("BEGIN:VCARD\rEND:VCARD\r\n").properties
+ assert_operator ProTacts::VCard::Parser::BrokenAssumption, :<, ProTacts::VCard::Parser::ParseError
+ assert_kind_of ProTacts::VCard::Parser::BrokenAssumption, error("BEGIN:VCARD\rEND:VCARD\r\n")
+ end
+
+ # broke_assumption? separates the break worth reporting from an
+ # ordinary bad line and from a line that read nothing at all.
+ def test_only_a_broken_assumption_reads_as_one
+ assert_equal [false, false, false], lines("FN:A\r\n\r\nBDAY;=;:\r\n").map { it.broke_assumption? }
+ assert lines("FN:A\rNOTE:n\r\n").fetch(0).broke_assumption?
end
def test_a_line_that_will_not_read_carries_its_error_and_keeps_its_bytes
line = ProTacts::VCard::Parser.lines("BDAY;=;:\r\n").fetch(0)
- assert_kind_of ProTacts::VCard::ParseError, line.error
+ assert_kind_of ProTacts::VCard::Parser::ParseError, line.error
assert_nil line.property
assert_equal "BDAY;=;:\r\n", line.verbatim
end
- # The two reads' strictness split, pinned: parse declines the whole
- # card, lines carries the same line with its error — and the raise is
- # that error, not a fresh one.
- def test_parse_raises_where_lines_carries
- card = "FN:A\r\nBDAY;=;:\r\n"
- carried = ProTacts::VCard::Parser.lines(card).fetch(1)
-
- error = assert_raises(ProTacts::VCard::ParseError) { parse(card) }
+ # A line that will not read costs its own reading and nothing else:
+ # the lines around it read, so the failure never travels past the
+ # line it happened on.
+ def test_a_line_that_will_not_read_leaves_the_rest_alone
+ read = lines("FN:A\r\nBDAY;=;:\r\nUID:b\r\n")
- assert_equal carried.error.message, error.message
+ assert_equal ["FN", nil, "UID"], read.map { it.property&.name }
+ assert_equal [nil, ProTacts::VCard::Parser::ParseError, nil], read.map { it.error&.class }
end
# A blank line is a line — its bytes stay enumerable, so a rewrite
- # carrying lines does not drop them — while parse skips it, as the
- # grammar lets a card carry blank lines between properties.
- def test_a_blank_line_stays_a_line_but_parses_as_none
- lines = ProTacts::VCard::Parser.lines("FN:A\r\n\r\nUID:b\r\n")
+ # carrying lines does not drop them — while it reads as no property,
+ # as the grammar lets a card carry blank lines between properties.
+ def test_a_blank_line_stays_a_line_but_reads_as_none
+ read = lines("FN:A\r\n\r\nUID:b\r\n")
- assert_equal ["FN:A\r\n", "\r\n", "UID:b\r\n"], lines.map(&:verbatim)
- assert_nil lines.fetch(1).property
- assert_nil lines.fetch(1).error
- assert_equal %w[FN UID], parse("FN:A\r\n\r\nUID:b\r\n").map { it.name }
+ assert_equal ["FN:A\r\n", "\r\n", "UID:b\r\n"], read.map(&:verbatim)
+ assert_nil read.fetch(1).property
+ assert_nil read.fetch(1).error
+ assert_equal %w[FN UID], properties("FN:A\r\n\r\nUID:b\r\n").map { it.name }
end
## Refusals
- def test_a_line_without_a_colon_raises
- error = assert_raises(ProTacts::VCard::ParseError) { parse(card("FN")) }
-
- assert_match(/expected ':'/, error.message)
+ def test_a_line_without_a_colon_does_not_read
+ assert_match(/expected ':'/, error(card("FN")).message)
end
- def test_a_line_with_no_name_raises
- assert_raises(ProTacts::VCard::ParseError) { parse(card(":value")) }
+ def test_a_line_with_no_name_does_not_read
+ assert_kind_of ProTacts::VCard::Parser::ParseError, error(card(":value"))
end
# vCard 2.1 allowed `TEL;HOME:...`; the 3.0 grammar does not, and
# guessing which parameter was meant would invent data.
- def test_a_parameter_without_a_value_raises
- error = assert_raises(ProTacts::VCard::ParseError) { parse(card("TEL;HOME:+1-555-1234")) }
-
- assert_match(/HOME has no value/, error.message)
+ def test_a_parameter_without_a_value_does_not_read
+ assert_match(/HOME has no value/, error(card("TEL;HOME:+1-555-1234")).message)
end
- def test_bytes_that_are_not_a_card_raise
- assert_raises(ProTacts::VCard::ParseError) { parse("{\"json\": true}\r\n") }
+ def test_bytes_that_are_not_a_card_do_not_read
+ assert_kind_of ProTacts::VCard::Parser::ParseError, error("{\"json\": true}\r\n")
end
## Property tests
@@ -263,9 +277,9 @@ class VCardParserTest < Minitest::Test
value = tc.draw(text(max_size: 300))
line = ProTacts::VCard.fold("NOTE:#{ProTacts::VCard.escape(value)}")
- properties = ProTacts::VCard::Parser.parse("#{line}\r\n")
- raise "expected one property, got #{properties.length}" unless properties.length == 1
- raise "value did not survive" unless unescape(properties.first.value) == normalize(value)
+ parsed = properties("#{line}\r\n")
+ raise "expected one property, got #{parsed.length}" unless parsed.length == 1
+ raise "value did not survive" unless unescape(parsed.first.value) == normalize(value)
end
end
@@ -278,7 +292,7 @@ class VCardParserTest < Minitest::Test
values = tc.draw(arrays(token, min_size: 1, max_size: 4))
line = "TEL#{values.map { ";#{name}=#{it}" }.join}:+1-555-1234"
- property = ProTacts::VCard::Parser.parse("#{line}\r\n").first
+ property = properties("#{line}\r\n").first
raise "parameters did not survive" unless property.parameters == values.map { [name, it] }
end
end
diff --git a/test/test_helper.rb b/test/test_helper.rb
index 3f00b0d..29cb674 100644
--- a/test/test_helper.rb
+++ b/test/test_helper.rb
@@ -49,3 +49,25 @@ require "minitest/autorun"
# Standing in for config.ru, which never runs here: build the store and
# hand it to the app.
ProTacts::Web.store = FixtureData.install(data_dir)
+
+# Sentry's capture_message, made observable for the length of a block and
+# restored after, so what one test captures cannot leak into the next.
+# Sentry is never initialized under test, so the original is inert
+# anyway — this only makes it observable. Returns what was captured.
+#
+# Here rather than in one test class because the reports are raised from
+# two layers: the web's PUT reports a broken parser assumption, the store
+# reports what its birthday model could not recompose.
+module CapturingSentry
+ def capturing_sentry
+ messages = []
+ original = Sentry.method(:capture_message)
+ Sentry.define_singleton_method(:capture_message) { |message, **| messages << message }
+ begin
+ yield messages
+ ensure
+ Sentry.define_singleton_method(:capture_message, original)
+ end
+ messages
+ end
+end