Validate DDL column options and PDO DSN parameters - #79
Merged
simon-mundy merged 18 commits intoSep 4, 2026
Conversation
simon-mundy
commented
Aug 31, 2026
simon-mundy
commented
Member
- Reject charset/collate values that are not bare names, restrict COLUMN_FORMAT and STORAGE to enum keywords, and reject DSN control characters in connection parameters.
- Consolidates column option handling into ColumnOptionTrait so options are validated in one place.
- Minor code cleanup
Reject charset/collate values that are not bare names, restrict COLUMN_FORMAT and STORAGE to enum keywords, and reject DSN control characters in connection parameters. Consolidates the triplicated column option handling into ColumnOptionTrait so each option is validated in one place.
The DSN parameter validation is generic to all semicolon-delimited PDO DSN formats and has no MySQL-specific dependencies, so it can move to php-db/phpdb once a second PDO driver package needs it. Signed-off-by: Simon Mundy <simon.mundy@peptolab.com>
Brings in the mago reformat (PR php-db#57), PHPUnit 12 migration, and phpdb 0.6 compatibility. Conflicts resolved by keeping the ColumnOptionTrait consolidation and DSN parameter validation from this branch while adopting the incoming type improvements, null guards, and adapterPlatform naming. Signed-off-by: Simon Mundy <simon.mundy@peptolab.com>
Formats the files this branch touched plus one straggler the mago-format-reformat pass missed (IntegrationTestStoppedListener). Whitespace, member ordering, and trailing commas only - no behavior change. Signed-off-by: Simon Mundy <simon.mundy@peptolab.com>
The merge combined this branch's test-prefixed additions with the PHPUnit 12 attribute-style renames of the same base scenarios from 0.5.x, leaving 11 duplicate tests. Keeps the unique additions - the SQL-injection rejection tests, the DDL option matrix tests, and the column_format alias test - renamed to the attribute convention. Signed-off-by: Simon Mundy <simon.mundy@peptolab.com>
Replaces the formatter's paren-wrapped multi-line return expressions with separate statements: the enums resolve tryFrom() into a variable and throw on null, and resolveAfterOption() returns early for other options. Signed-off-by: Simon Mundy <simon.mundy@peptolab.com>
Asserts ColumnInterface and option-array types locally where the upstream properties are untyped arrays, makes the trait's platform parameter non-nullable (callers already guard), casts option values fed to quoteValue()/quoteIdentifier(), and rewrites the offset-bump loop to iterate the offsets instead of variable-indexing into the array shape, which mago cannot prove defined. Corrects the inherited processChangeColumns() return docblock to the actual pair-list shape. Carries the decorators' complexity suppressions onto the trait the consolidation moved that complexity into, and regenerates the analyze baseline with mago 1.47, whose changed type phrasing had orphaned the old entries (the SelectDecorator false positive and 39 stale entries). The baseline contains no entries for the code this branch adds. Signed-off-by: Simon Mundy <simon.mundy@peptolab.com>
Upstream phpdb 0.6.x-dev added getQueryResult() to ResultInterface after this repo's lock (e037464 -> b57f549), which fataled the CI latest/lowest dependency legs - both resolve the dev branch to its tip. Mirrors the upstream Pdo\Result implementation, bumps the lock to the same tip so all three legs test one phpdb revision, and regenerates the analyze baseline for the vendor's shifted types. Signed-off-by: Simon Mundy <simon.mundy@peptolab.com>
The CoversMethod attributes for getSqlInsertOffsets(), compareColumnOptions(), and normalizeColumnOption() still named the decorator classes, but the ColumnOptionTrait consolidation moved those methods into the trait, so php-code-coverage rejects them as targets - 78 PHPUnit warnings that fail the coverage CI leg under failOnWarning. Replaces them with CoversTrait(ColumnOptionTrait), which also credits the trait's other methods these tests exercise. Signed-off-by: Simon Mundy <simon.mundy@peptolab.com>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
The injection-rejection and option matrix tests drive both getOptionValue() paths of ColumnFormatEnum and StorageEnum, but with requireCoverageMetadata that execution earns no credit until a test class claims the enums - they reported 0% despite full exercise. Declares CoversClass for both on the two decorator test classes, taking each enum from 0/9 to 9/9 covered statements. Signed-off-by: Simon Mundy <simon.mundy@peptolab.com>
Brings Connection, Driver, Result, Statement, and the Pdo counterparts to full statement coverage. Adds unit tests for the PDO-injected constructor path, failed-query and throwing-lastInsertId fallbacks, driverless execute, and unconnected rollback; integration tests for repeat connects, driver_options handling, invalid-SQL execution and preparation (with mysqli report mode restored), schema lookup without a selected database, statement re-preparation, failing prepared inserts, no-metadata statement results, unbuffered exhaustion, direct next(), undetermined buffering, and Pdo auto-connecting getCurrentSchema. Server-failure paths that first looked untestable are covered for real by killing the connection's thread from a second connection: a wide streaming result set (REPEAT over a self-join, so it always exceeds the socket buffers) provokes the fetch() error mid-iteration, and the same technique reaches getCurrentSchema's failed-query throw. The entry guard in loadDataFromMysqliStatement() is removed rather than excluded: both call sites dispatch on instanceof mysqli_stmt, unserialize() enforces the typed property union, and instanceof cannot be spoofed, so the check was unreachable from any path. A narrowing @var assignment documents the invariant for static analysis instead. The remaining structurally unreachable branches - a connect_error check a successful real_connect() cannot leave set, fetch_row() on an already-buffered result, the ext-mysqli check the suite itself requires, and the Pdo analyzer-narrowing guards - carry @codeCoverageIgnore blocks stating why. The annotation must sit alone on its comment line; a trailing reason stops php-code-coverage from recognising it. Project statement coverage: 91.05% -> 95.37%. Metadata/Source.php is the only file with uncovered lines left. Signed-off-by: Simon Mundy <simon.mundy@peptolab.com>
Deletes the dead Pdo Driver feature branch and the unreachable second connect_error check, replaces the null-platform and null-resource guards with bare @var narrowings, rewrites the PDOException code fallback as a ternary, and strips explanatory prose from the surviving pragmas. Only the ext-mysqli check and the buffered fetch_row() failure branch remain excluded. Signed-off-by: Simon Mundy <simon.mundy@peptolab.com>
Slot, emission order and SQL template for every column option now come from one COLUMN_OPTIONS constant in ColumnOptionTrait, replacing the separate sort-order property and the slot literals repeated in each match arm. The match only resolves the value to substitute, so an extra resolver such as AFTER returns the quoted identifier alone and can no longer disagree with the table about where it belongs. Keyword options lean on the enum directly: getColumnOptionKeyword() guards the type, then lets BackedEnum::from() validate and throw its own ValueError. The duplicated getOptionValue() helpers on ColumnFormatEnum and StorageEnum are removed. Tests for unknown keywords now expect ValueError, and non-string keyword values are covered for CREATE TABLE, ADD COLUMN and CHANGE COLUMN.
CI installs the latest Mago, which moved from 1.47.4 to 1.47.5 since the last green run. The new release reports the Metadata\Source $data shape mismatch under invalid-property-assignment-value instead of property-type-coercion, and adds a possibly-undefined index warning on the mysqli Result's bind-value dereference. Both are pre-existing and unrelated to this branch; the baseline is regenerated so it matches under either release.
The dereference loop indexed keys and values by a shared counter, which static analysis could not prove in range and which sat behind five baseline entries. The values array is filled to the length of keys when the bindings are created, so array_combine() expresses the same operation directly and the baseline entries are dropped.
Narrowing the closure's return type to ColumnFormatEnum|StorageEnum lets the analyzer see that the case value is a string, so the cast on the return is no longer needed and the BackedEnum import goes with it.
tyrsson
approved these changes
Sep 4, 2026
tyrsson
left a comment
tyrsson
left a comment
Member
There was a problem hiding this comment.
Lets leave the branch in place for the moment after it merges just in case. Github has had so many issues lately I'd feel better in case any of its needed later for revert/cherry pick.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.