feat: punycode email domains before sending (closes #26)#27
Merged
Conversation
The Vatly API requires email domains in Punycode (ASCII) form per RFC 3492 and returns 422 `email_domain_not_ascii` for raw Unicode. The SDK was passing addresses through verbatim, forcing every caller to know about IDN. Adds `Vatly\API\Support\IdnEmail::toAscii()` (splits on the last `@`, runs the domain through `idn_to_ascii` with the UTS46 variant, fail-open on empty/failed conversion). Wires it into `BaseEndpoint::parseRequestBody` via a small recursive walker so every payload — current and future — gets normalized at a single point. Covers top-level `email`, nested `billingAddress.email`, and any other `email` key downstream packages (vatly-fluent-php, vatly-laravel) construct. Local-part is intentionally left untouched: there is no ASCII encoding for non-ASCII local-parts (Punycode is a DNS-label spec), and the API explicitly does not support EAI/SMTPUTF8 (openapi.yaml line 84). A non-ASCII local-part should surface as the server's validation error rather than be silently mangled. Declares `ext-intl` in composer.json. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…tadata Two follow-ups from PR review: 1. `SubscriptionEndpoint::updateBilling` and `OrderEndpoint::requestAddressUpdateLink` were hand-rolling `json_encode($data)` instead of going through `parseRequestBody()`, so they bypassed the email normalization added in the previous commit. Both endpoints accept `billingAddress.email` per the OpenAPI spec, so an IDN domain still triggered the same 422 the patch was meant to prevent. Route them through `parseRequestBody()` like every other mutating call. 2. The recursive payload walker was rewriting any nested key named `email`, including inside `metadata`. The OpenAPI spec defines `metadata` as opaque application-defined key/value data, so the SDK must not mutate values nested there — a caller using `metadata.email` for their own tagging would get their data silently changed. Skip recursion into `metadata` entirely. New tests: - `IdnEmailTest::normalize_payload_leaves_metadata_untouched` - `SubscriptionEndpointTest::update_billing_punycodes_email_domains_in_billing_address` - `OrderEndpointTest::request_address_update_link_punycodes_email_domains_in_billing_address` `TestHelpersEndpoint::simulatePaymentFailure` also hand-rolls `json_encode`, but its payload shape doesn't carry email, so leaving it alone to keep the diff minimal. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
4 tasks
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
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.
Summary
Closes #26.
The Vatly API requires email domains in Punycode (ASCII) form per RFC 3492 and returns
422 email_domain_not_asciifor raw Unicode (seeopenapi.yamllines 66–84). The SDK was passing addresses through verbatim, forcing every caller to know about IDN. This PR makes the SDK normalize transparently.What changed
src/API/Support/IdnEmail.php(new)toAscii(string $email): string— splits on the last@, runs the domain throughidn_to_ascii($domain, IDNA_NONTRANSITIONAL_TO_ASCII, INTL_IDNA_VARIANT_UTS46), fail-open if there's no@, empty domain, oridn_to_asciireturnsfalse. On ASCII input it's a no-op; onuser@münchen.deit returnsuser@xn--mnchen-3ya.de.normalizePayload(array): array— recursively walks a payload and appliestoAsciito any string value at a key namedemail.src/API/Endpoints/BaseEndpoint.phpIdnEmail::normalizePayload()once insideparseRequestBody(), right beforejson_encode. Single integration point covers every endpoint (current and future) and every nested address — no per-call-site changes, no risk of forgetting a new endpoint. Downstream packages (vatly-fluent-php,vatly-laravel) that hand arrays to this SDK pick it up automatically.composer.json— addsext-intltorequire.Why local-part is untouched
I had to push back on the original framing here. There is no ASCII encoding for non-ASCII local-parts — Punycode (RFC 3492) is defined for DNS labels only. The only standardized path for Unicode local-parts is SMTPUTF8 (RFC 6531), passthrough UTF-8 on the wire. And the API explicitly does not support EAI/SMTPUTF8 per
openapi.yamlline 84:So if a caller passes
josé@example.com, the SDK passing it through unchanged is correct behavior — the server returns a meaningful validation error rather than the SDK silently mangling someone's address. This matches the original issue's framing ("local-part stays as UTF-8") and matches the spec.Behavior
user@example.comuser@example.com(passthrough)user@münchen.deuser@xn--mnchen-3ya.debilling@müller.debilling@xn--mller-kva.de"foo@bar"@münchen.de"foo@bar"@xn--mnchen-3ya.de(last-@split)josé@münchen.dejosé@xn--mnchen-3ya.de(domain only; server will 422 on local-part)not-an-emailnot-an-email(no@, returned unchanged)user@user@(empty domain, fail-open)Tests
tests/Support/IdnEmailTest.php— 8 unit tests covering ASCII passthrough, IDN conversion, no-@fallback, empty-domain fallback, last-@split, Unicode local-part untouched, nested payload walking, non-stringemailvalues left alone.tests/Endpoints/CustomersEndpointTest.php— 1 integration test assertinguser@münchen.degoes out on the wire asuser@xn--mnchen-3ya.devia the real endpoint stack.Test plan
vendor/bin/phpunit— 105/105 pass (was 96, +9 new)vendor/bin/phpstan analyse src tests --memory-limit=512M— cleanvendor/bin/php-cs-fixer fix --allow-risky=yes --dry-run— cleanAcceptance criteria (from #26)
email(orbillingAddress.email) field route through it@fallback, failed conversion fallbackcomposer.jsondeclaresext-intl