Skip to content

fix(turba): enforce attribute constraints on every write path - #74

Merged
ralflang merged 1 commit into
FRAMEWORK_6_0from
fix/enforce-attribute-constraints
Nov 2, 2026
Merged

ralflang merged 1 commit into
FRAMEWORK_6_0from
fix/enforce-attribute-constraints

Conversation

@TDannhauer

@TDannhauer TDannhauer commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Enforces the maxlength / regex constraints from attributes.php centrally in Turba_Driver::add() and save(), so every write path (web form, contacts/* API, ActiveSync, CardDAV, vCard/CSV import) rejects invalid values the same way instead of failing or truncating in the database.
  • New Turba_Exception_InvalidAttribute names the offending attribute and implements Horde\Exception\InvalidDataThrowable, so protocol handlers can report "client data rejected" instead of "server error".
  • Widens only the post office box columns (a genuine schema bug: VARCHAR(10) cannot hold "P.O. Box 12345"); postal code columns stay at 10, now protected by the validator.

Fixes #73.

Motivation

A user entered postal code and city into the ZIP field on a phone. Turba stored nothing, PostgreSQL logged SQLSTATE[22001] value too long for type character varying(10), ActiveSync reported a generic server error, and the device resent the same contact on every Sync. On MySQL in non-strict mode the same input would have been silently truncated. The web form would have rejected it with a clear message — because Horde_Form_Type_text::isValid() was the only place the attribute constraints were checked.

Raising the column width alone would just move the limit; the defect is that the constraints are not enforced on the write path. See the rewritten #73 for the full analysis.

Changes

  • src/AttributeValidator.php (new, Horde\Turba\AttributeValidator): checks params.maxlength (via HordeString::length) and params.regex for every attribute present in a value array. Skips __* keys, undefined attributes and empty values. Error messages reuse the translated Horde_Form strings ("Value is over the maximum length of %d." / "You must enter a valid value."), prefixed with the attribute label — no new translatable strings.
  • lib/Exception/InvalidAttribute.php (new): extends Turba_Exception (existing catch (Turba_Exception) blocks keep working) implements InvalidDataThrowable; getAttribute().
  • lib/Driver.php: accepts an attribute_validator parameter in the constructor (consumed, not kept in $_params), _validateAttributes() restricted to the attributes the source stores ($this->fields); called in add() after ensureAttributes() and at the start of save(). Drivers constructed without a validator behave as before.
  • lib/Factory/Driver.php: passes the validator from the injector when bound.
  • lib/Application.php: binds AttributeValidator in _init() from the loaded $attributes, so local attributes.php overrides and hooks are honoured.
  • migration/14_turba_upgrade_addresslengths.php (new): object_homepob, object_workpob, object_otherpob → string(255); down() restores 10. Column existence is guarded.
  • config/attributes.php: maxlength 10 → 255 for homePOBox, workPOBox, otherPOBox, commonPOBox (size unchanged).
  • doc/UPGRADING.rst: documents the enforced constraints and the migration.
  • test/Turba/Unit/AttributeValidatorTest.php (new).

Behaviour change

Writes that previously failed in the database (PostgreSQL) or were truncated (MySQL non-strict) now throw Turba_Exception_InvalidAttribute with a message such as Home Postal Code: Value is over the maximum length of 10.. The web form is unaffected (it validated the same rules before the driver was reached). Writes through the API that relied on silent truncation will now fail — intentionally.

Exceptions

No new catch blocks. Turba_Exception_InvalidAttribute is a Turba_Exception, so existing handlers (API, ActiveSync driver, CardDAV, import) continue to work; the ActiveSync driver in horde/core additionally recognises the InvalidDataThrowable marker.

Dependencies

Test plan

  • AttributeValidatorTest (6 tests): maxlength, regex, label fallback, __* / undefined / empty values skipped, valid values pass
  • Driver end-to-end against an in-memory SQLite Turba_Driver_Sql: add()/save() with a 13-character postal code throw Turba_Exception_InvalidAttribute (getAttribute() === 'homePostalCode'), valid values are stored, attribute_validator does not leak into getParams()
  • Migration chain 1→14 and 14→13 against SQLite (turba_objects.object_homepob limit 10 → 255 → 10)
  • Runtime check in a deployment: Turba_Registry::appInit('turba') binds the validator; Turba_Factory_Driver::create() hands it to Turba_Driver_Sql; the loaded attributes.php rejects 12345 Example for homePostalCode and accepts a long P.O. box
  • horde-db-migrate turba on PostgreSQL / MySQL
  • Sync a contact with an over-long ZIP from a device: contact rejected, log names the attribute, device stops resending (with the companion PRs)

The maxlength and regex parameters of attributes.php were only checked
by the Horde_Form text type in the web edit form. Turba_Driver::add()
and save() handed raw values to the backend, so API, ActiveSync,
CardDAV and import writes ended in a database error (PostgreSQL:
SQLSTATE 22001 "value too long for type character varying(10)") or in
silent truncation (MySQL, non-strict mode).

Add Horde\Turba\AttributeValidator, bound in Turba_Application::_init()
from the loaded attribute definitions and handed to drivers by
Turba_Factory_Driver. Turba_Driver::add() and save() validate the
attributes the source stores and throw Turba_Exception_InvalidAttribute
(extends Turba_Exception, implements Horde\Exception\InvalidDataThrowable,
exposes getAttribute()) naming the offending field. Messages reuse the
translated Horde_Form strings.

The post office box columns (object_homepob, object_workpob,
object_otherpob) were a genuine schema bug - "P.O. Box 12345" never fit
into VARCHAR(10). Migration 14 widens them to 255 and attributes.php
raises their maxlength. Postal code columns keep their width of 10; the
limit is now enforced by the validator instead of the database.

Fixes #73
@ralflang
ralflang merged commit 05841fe into FRAMEWORK_6_0 Nov 2, 2026
1 check failed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Attribute constraints (maxlength/regex) are only enforced in the web form; API/ActiveSync/CardDAV writes fail in the database or truncate silently

2 participants