feat: add email adapter DSN parsing#117
feat: add email adapter DSN parsing#117TorstenDittmann wants to merge 1 commit intofeat-multiple-adapters-failoverfrom
Conversation
Greptile SummaryThis PR adds Confidence Score: 4/5Safe to merge after addressing the previously flagged failing test assertion and the minor validation gaps. No new P0/P1 findings from this pass. Score is held at 4 rather than 5 because open items from prior review rounds — the test_rejects_malformed_smtp_dsn assertion mismatch that will cause a test failure, the missing TCP port-range guard, and the counter-intuitive query-string port precedence — are still present and unresolved. tests/Messaging/Adapter/Email/DsnTest.php (wrong exception message assertion), src/Utopia/Messaging/Adapter/Email.php (port range and precedence) Important Files Changed
Reviews (2): Last reviewed commit: "feat: add email adapter DSN parsing" | Re-trigger Greptile |
| $port = self::parseIntOption( | ||
| value: $query['port'] ?? ($parts['port'] ?? ($scheme === 'smtps' ? 465 : 25)), | ||
| option: 'port' | ||
| ); |
There was a problem hiding this comment.
Query-string port silently overrides URL port
The precedence $query['port'] > $parts['port'] > scheme-default means smtp://host:587?port=25 resolves to port 25. Most DSN parsers treat the URL authority port as authoritative. This is a subtle gotcha worth a brief inline comment, or consider flipping the precedence to URL port > query port > scheme default.
b62347d to
32c3976
Compare
Summary
Email::fromDsn()to create email adapters from DSN stringsTesting