Contacts sync: a card with an FN or ORG over 200 characters fails every run on PostgreSQL, and adoption leaves a new contact each time #20

New issue

Closed

opened 2026-10-01 10:47:51 +00:00 by tiagoagueda · 1 comment

tiagoagueda commented

2026-10-01 10:47:51 +00:00

Owner

Found on 2026-09-29 in a read-only review of postulo-dav at 97abc9779, checking every value apply_card writes against the length of the field it lands in.

apply_card cuts the role, the email, the number and the URL to their fields' lengths, but not the name and not the company. Adoption creates the contact, applies the card and binds the link as three separate steps with no transaction around them. On PostgreSQL an over-long value fails the save, the run ends there, and the contact already created stays behind without a link, so the next run adopts the same card again. Reasoned from the code; not run against PostgreSQL. On SQLite, which does not enforce max_length, the same card was adopted with a 250-character name stored in a 200-character field.

Where: src/postulo_dav/sync.py:148 (contact.name = card.name or contact.name, not cut), src/postulo_dav/sync.py:152-153 and :536 (card.org passed whole to get_or_create_company), src/postulo_dav/sync.py:536-549 (create, apply, bind), src/postulo_dav/sync.py:400-414 (sync, contacts then interviews with nothing between). Contact.name and Company.name are max_length=200 (jobs/models.py:622, :245).
What happens:
1. Adoption, long FN. Contact.objects.create(..., name=card.name[:200]) succeeds. apply_card then assigns the uncut name and contact.save() raises. The scheduler runs syncs outside any transaction, so the created contact is committed. SyncLink.bind never runs, the card is still unlinked on the next run, and each run adds another contact before failing.
2. Long ORG, or a long FN on a card already linked. get_or_create_company or contact.save() raises; no duplicate, but the run fails every time.
3. In each case the exception leaves sync before the calendar, so interviews stop syncing until somebody shortens the card. Related to #1 (closed), which asked that a failure in one half not sink the other; that part was not done.
4. Under Sync now the run is inside the request's transaction (ATOMIC_REQUESTS, config/settings/base.py:140). A failed statement aborts that transaction on PostgreSQL, so the core's own connection.save in sync_connection (plugins/syncing.py:93), which is documented as never raising, would fail as well.
Why it matters: one unusual card stops the whole connection, contacts and calendar, and on the scheduler it leaves a growing pile of unlinked contacts.
Fix: cut card.name and card.org to 200 in apply_card and at sync.py:536, as the other fields already are. Wrap the handling of one card (for adoption: create, apply, bind) in transaction.atomic(), catch a database or validation error there, and turn it into a note that names the card, so one card neither leaves a half-made contact nor ends the run. Run the contacts half and the interviews half so that an error in the first is reported and the second still runs. Related to #7, whose point 5 asks the same for a server's refusal of one item; this is the database's refusal.

Done when

tests/test_dav.py has a test that adopts a card whose FN and ORG are each 250 characters: one contact, one link, both names at most 200 characters, and a second run that says nothing to do.
tests/test_dav.py has a test in which applying one card raises (for example apply_card patched to raise for that card): no contact is left without a link, the report carries a note, the other cards are adopted and an interview is still pushed in the same run.

Found on 2026-09-29 in a read-only review of postulo-dav at 97abc9779, checking every value `apply_card` writes against the length of the field it lands in. `apply_card` cuts the role, the email, the number and the URL to their fields' lengths, but not the name and not the company. Adoption creates the contact, applies the card and binds the link as three separate steps with no transaction around them. On PostgreSQL an over-long value fails the save, the run ends there, and the contact already created stays behind without a link, so the next run adopts the same card again. Reasoned from the code; not run against PostgreSQL. On SQLite, which does not enforce `max_length`, the same card was adopted with a 250-character name stored in a 200-character field. - **Where:** `src/postulo_dav/sync.py:148` (`contact.name = card.name or contact.name`, not cut), `src/postulo_dav/sync.py:152-153` and `:536` (`card.org` passed whole to `get_or_create_company`), `src/postulo_dav/sync.py:536-549` (create, apply, bind), `src/postulo_dav/sync.py:400-414` (`sync`, contacts then interviews with nothing between). `Contact.name` and `Company.name` are `max_length=200` (`jobs/models.py:622`, `:245`). - **What happens:** 1. *Adoption, long `FN`.* `Contact.objects.create(..., name=card.name[:200])` succeeds. `apply_card` then assigns the uncut name and `contact.save()` raises. The scheduler runs syncs outside any transaction, so the created contact is committed. `SyncLink.bind` never runs, the card is still unlinked on the next run, and each run adds another contact before failing. 2. *Long `ORG`, or a long `FN` on a card already linked.* `get_or_create_company` or `contact.save()` raises; no duplicate, but the run fails every time. 3. In each case the exception leaves `sync` before the calendar, so interviews stop syncing until somebody shortens the card. Related to #1 (closed), which asked that a failure in one half not sink the other; that part was not done. 4. Under *Sync now* the run is inside the request's transaction (`ATOMIC_REQUESTS`, `config/settings/base.py:140`). A failed statement aborts that transaction on PostgreSQL, so the core's own `connection.save` in `sync_connection` (`plugins/syncing.py:93`), which is documented as never raising, would fail as well. - **Why it matters:** one unusual card stops the whole connection, contacts and calendar, and on the scheduler it leaves a growing pile of unlinked contacts. - **Fix:** cut `card.name` and `card.org` to 200 in `apply_card` and at `sync.py:536`, as the other fields already are. Wrap the handling of one card (for adoption: create, apply, bind) in `transaction.atomic()`, catch a database or validation error there, and turn it into a note that names the card, so one card neither leaves a half-made contact nor ends the run. Run the contacts half and the interviews half so that an error in the first is reported and the second still runs. Related to #7, whose point 5 asks the same for a server's refusal of one item; this is the database's refusal. ## Done when - `tests/test_dav.py` has a test that adopts a card whose `FN` and `ORG` are each 250 characters: one contact, one link, both names at most 200 characters, and a second run that says `nothing to do`. - `tests/test_dav.py` has a test in which applying one card raises (for example `apply_card` patched to raise for that card): no contact is left without a link, the report carries a note, the other cards are adopted and an interview is still pushed in the same run.

tiagoagueda added this to the 0.6.0 milestone

2026-10-01 10:47:51 +00:00

tiagoagueda added the

bug

label

2026-10-01 10:47:51 +00:00

tiagoagueda referenced this issue

2026-10-17 10:39:50 +00:00

SEC2-057: A URL pulled from a card is stored as a contact's web link with no scheme check, and pages render it as href #32

tiagoagueda referenced this issue

2026-10-17 10:39:52 +00:00

SEC2-059: One malformed date or an over-long remote identifier fails the whole run, on every run #34