Skip to content

Fix vCard import validation before replacing existing links - #1004

Open
sridharkalaibala wants to merge 2 commits into
LinkStackOrg:mainfrom
sridharkalaibala:fix/vcard-import-validation
Open

sridharkalaibala wants to merge 2 commits into
LinkStackOrg:mainfrom
sridharkalaibala:fix/vcard-import-validation

Conversation

@sridharkalaibala

@sridharkalaibala sridharkalaibala commented Sep 13, 2026

Copy link
Copy Markdown

Importing an exported vCard currently fails because its JSON contact payload is validated as a URL. Existing links have already been deleted by that point. Validate link payloads before any profile or link writes, accepting JSON objects for type: vcard while retaining the existing URL validation for other types.

Fixes #976. This concerns importing contact data; it is separate from #962's vCard generation change.

Validation:

  • 13 controller/database tests, 35 assertions pass using Laravel, SQLite in memory, the repository's migrations and ButtonSeeder, and real UploadedFile requests. Covers mixed URL/vCard imports, preserving quoted contact values, malformed JSON and non-object contact values, malformed export structures, invalid ordinary URLs, preservation of existing links/profile name on validation failures, and intentionally empty exports.

  • Against original main, the initial nine cases produced seven failures and one error; only the ordinary-link control passed. A separate original-code reproduction imported one vCard, returned an error, and left zero existing links.

  • PHP syntax checks and git diff --check pass.

Validation environment and limits:

  • Removed the suite entry for the absent tests/Unit directory. The default php vendor/bin/phpunit command now passes the committed suite: 13 tests, 35 assertions.

  • The checked-in Composer lock is inconsistent with the manifest (missing collision and yaml). Following the contribution setup's composer update resolved dependencies in an isolated Linux copy, using PHP 8.3.6, Laravel 9.52.22 and PHPUnit 9.6.36. No dependency or lockfile changes are included. Composer reported four advisories affecting one installed package.

  • The committed tests exercise the controller/database directly. Additional isolated HTTP-kernel validation passes 14 tests / 64 assertions through the registered import route with web/auth middleware, including a guest request rejected without changing existing links. The original controller fails 11 of those 14 cases. This additional probe is not committed; its fixture seeds the in-memory database, supplies a test-only encryption key, and loads installed routes after bootstrapping installer mode, restoring the installer marker afterward. This is not a browser login, CSRF-token enforcement (Laravel testing bypass), or MySQL test. Tests use separate processes because existing application config declares global functions on boot.

  • Prevalidation protects against the covered payload-validation failures. It does not make file/avatar changes transactional or provide rollback for every possible later database failure. Detailed validation of every vCard field is outside this change.

Prepared and tested with OpenAI Codex assistance. Based on current default main; contribution instructions still refer to master.

@sridharkalaibala
sridharkalaibala marked this pull request as ready for review September 14, 2026 08:16
@sridharkalaibala

Copy link
Copy Markdown
Author

Marked this ready for review after resolving the default PHPUnit invocation in a2cddf5. The committed suite now runs with php vendor/bin/phpunit: 13 tests / 35 assertions pass. Additional isolated HTTP-kernel checks through the registered import route and web/auth middleware pass 14 tests / 64 assertions, including rejecting a guest request without changing existing links; the original controller fails 11 of those cases.

The body describes the fixture and dependency setup, and distinguishes these checks from browser login, MySQL, CSRF-token enforcement and full rollback guarantees. The production change remains scoped to vCard validation and validating link payloads before mutation.

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.

Import failing when importing a vcard-button

1 participant