store: a BDAY sharing its line stays put, reported
bday_of replaces the positional reads: a lone CR can pack a second
content line into one physical line, and properties.first would move
the birthday out while silently dropping the bytes riding beside it.
The migration makes the same move and follows it.

Assisted-by: GLM-5.3 via pi
change xmwtkkwylwlsxlvzkttopqsuoqnvtkoo
commit dbc32161353a5414e986851fa9eba7f822b9ee95
author Alpha Chen <alpha@kejadlen.dev>
date
parent prnzowqk
diff --git a/db/migrations/002_birthdays.rb b/db/migrations/002_birthdays.rb
index 8b62f7b..2b136f6 100644
--- a/db/migrations/002_birthdays.rb
+++ b/db/migrations/002_birthdays.rb
@@ -44,8 +44,14 @@ Sequel.migration do
       # card's BDAY lines take — only the modeled single line moves.
       case ProTacts::VCard.new(row.fetch(:vcard)).lines.partition { it.names?("BDAY") }
       in [[line], others]
-        property = line.properties.first
-        birthday = property && ProTacts::Birthday.from_property(property)
+        # Only a line holding exactly one BDAY property can move it
+        # out: the line's bytes move as one, and a lone CR can pack a
+        # second content line into them.
+        next unless line.properties.one?
+        property = line.properties.fetch(0)
+        next unless property.name.casecmp?("BDAY")
+
+        birthday = ProTacts::Birthday.from_property(property)
         next if birthday.nil?
 
         self[:cards].where(id: row.fetch(:id)).update(vcard: others.map(&:verbatim).join)
diff --git a/lib/pro_tacts/store.rb b/lib/pro_tacts/store.rb
index 4911c71..12238d6 100644
--- a/lib/pro_tacts/store.rb
+++ b/lib/pro_tacts/store.rb
@@ -207,7 +207,8 @@ module ProTacts
       # a submitted card's BDAY lines can take. One line that reads as
       # a modeled birthday moves out of the card and into the model;
       # any other BDAY — the vCard 4.0 forms, a foreign sentinel, a
-      # line that will not parse, more than one — is data the model
+      # line that will not parse, a BDAY sharing its line's bytes with
+      # another, more than one — is data the model
       # cannot recompose and stays in the card byte for byte, with the
       # model emptied so nothing composes a second BDAY beside it
       # (RFC 6352 section 6.3.2.2). No BDAY at all is a client's
@@ -223,7 +224,7 @@ module ProTacts
         case VCard.new(vcard).lines.partition { it.names?("BDAY") }
         in [[line], others]
           report_unrecognized_bday_lines([line])
-          property = line.properties.first
+          property = bday_of(line)
           birthday = property && Birthday.from_property(property)
           [birthday, birthday ? others.map(&:verbatim).join : vcard]
         in [[], _]
@@ -379,6 +380,21 @@ module ProTacts
       row && row.fetch(:vcard).to_s
     end
 
+    # The BDAY a decision may be made of: the line's one property,
+    # when the line holds exactly one and it names BDAY. Found by name,
+    # not position, and only when alone, because the line is the unit a
+    # rewrite or a move to the model carries — a BDAY sharing its bytes
+    # with another content line (a lone CR packs two into one physical
+    # line) cannot leave without taking the rest with it. Nil for any
+    # other shape, the empty line a failed read leaves included.
+    #: (VCard::Line line) -> VCard::Property?
+    def bday_of(line)
+      return nil unless line.properties.one?
+
+      property = line.properties.fetch(0)
+      property.name.casecmp?("BDAY") ? property : nil
+    end
+
     # A stored card's BDAY lines in two piles: the verbatim lines a
     # rewrite carries across — values no client renders — and the ones
     # it drops unwitnessed, which no client renders and no whitelist
@@ -390,11 +406,11 @@ module ProTacts
 
       bdays, = VCard.new(vcard).lines.partition { it.names?("BDAY") }
       carried, rest = bdays.partition { |line|
-        property = line.properties.first
+        property = bday_of(line)
         property && Birthday.unrendered_value?(property.value)
       }
       lost = rest.reject { |line|
-        property = line.properties.first
+        property = bday_of(line)
         property && Birthday.rendered?(property)
       }
       [carried.map(&:verbatim), lost]
@@ -409,7 +425,7 @@ module ProTacts
     #: (Array[VCard::Line] lines) -> void
     def report_unrecognized_bday_lines(lines)
       unrecognized = lines.count { |line|
-        property = line.properties.first
+        property = bday_of(line)
         property.nil? || (!Birthday.rendered?(property) && !Birthday.unrendered_value?(property.value))
       }
       return if unrecognized.zero?
diff --git a/test/pro_tacts/test_store.rb b/test/pro_tacts/test_store.rb
index 443855c..526f235 100644
--- a/test/pro_tacts/test_store.rb
+++ b/test/pro_tacts/test_store.rb
@@ -549,6 +549,23 @@ class StoreTest < Minitest::Test
   # rewrite that omits the BDAY carries them across or loses them
   # (docs/macos-contacts.md, "A birthday the client cannot render is
   # dropped from the card").
+  # A BDAY sharing its line's bytes with another content line — a lone
+  # CR packs two into one physical line — is not a birthday any single
+  # decision can be made of: taking the first property alone would drop
+  # the bytes beside it unwitnessed, so the line stays verbatim, the
+  # model empties, and the arrival is reported.
+  def test_a_bday_sharing_its_line_arrives_whole_and_reported
+    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|
+      messages = capturing_sentry { store.put("aiden", shared) }
+
+      assert_equal shared, store.contact("aiden").vcard
+      assert_nil birthday_row(store, "aiden")
+      assert_equal 1, messages.length
+    end
+  end
+
   def test_a_birthday_no_client_renders_survives_a_rewrite_that_drops_it
     ["BDAY:1985-04", "BDAY:1985", "BDAY:--04", "BDAY:---12"].each do |line|
       with_store({"aiden" => AIDEN.sub("END:VCARD\r\n", "#{line}\r\nEND:VCARD\r\n")}) do |store|
@@ -575,6 +592,21 @@ class StoreTest < Minitest::Test
     end
   end
 
+  # The rewrite's half of the same rule: a shared line is not carried
+  # (carrying it would carry its fellow bytes too), so dropping it is
+  # reported rather than silent.
+  def test_a_rewrite_dropping_a_shared_bday_line_is_reported
+    shared = AIDEN.sub("END:VCARD\r\n", "BDAY:1985-04\rNOTE:b\r\nEND:VCARD\r\n")
+    edited = AIDEN.sub("FN:Aiden", "FN:Aiden Smith")
+
+    with_store({"aiden" => shared}) do |store|
+      messages = capturing_sentry { store.put("aiden", edited) }
+
+      assert_equal edited, store.contact("aiden").vcard
+      assert_equal 1, messages.length
+    end
+  end
+
   # A submission carrying any BDAY replaces what was there, carried
   # line included: PUT is a whole-card replace, not a merge.
   def test_a_submitted_birthday_replaces_the_carried_line
@@ -698,12 +730,14 @@ class StoreTest < Minitest::Test
   # — is carried over by the same subtraction a write makes: modeled
   # forms move, unmodeled forms stay put.
   def test_the_migration_moves_a_modeled_bday_out_of_the_cards
+    shared = ZED.sub("UID:znorth", "UID:xavi").sub("END:VCARD\r\n", "BDAY:1985-04-12\rNOTE:b\r\nEND:VCARD\r\n")
     Dir.mktmpdir do |dir|
       path = Pathname.new(dir) / "contacts.db"
       Sequel.connect("sqlite://#{path}") do |db|
         Sequel::Migrator.run(db, ProTacts::Store::MIGRATIONS.to_s, target: 1)
         db[:cards].insert(id: "aiden", vcard: AIDEN_BORN)
         db[:cards].insert(id: "znorth", vcard: ZED.sub("END:VCARD\r\n", "BDAY:--0412\r\nEND:VCARD\r\n"))
+        db[:cards].insert(id: "xavi", vcard: shared)
       end
 
       ProTacts::Store.connect(path) do |store|
@@ -714,6 +748,12 @@ class StoreTest < Minitest::Test
         # Unmodeled: byte-identical, no birthday row beside it.
         assert_equal ZED.sub("END:VCARD\r\n", "BDAY:--0412\r\nEND:VCARD\r\n"), store.contact("znorth").vcard
         assert_nil birthday_row(store, "znorth")
+
+        # A BDAY sharing its line's bytes stays put: the line moves as
+        # one or not at all, where the first property alone would have
+        # moved the birthday and dropped the NOTE unwitnessed.
+        assert_equal shared, store.contact("xavi").vcard
+        assert_nil birthday_row(store, "xavi")
       end
     end
   end