The registrable-suffix check reads more spec-correct than the old topDomainOf(...) comparison — thanks for tightening this. Builds fine here.

Small robustness note: InternetDomainName.from(rpId) throws IllegalArgumentException on a malformed rpId, and it isn't wrapped, so a bad rpId would surface as a crash rather than a clean NOT_ALLOWED_ERR. Might be worth a runCatching { InternetDomainName.from(rpId).isPublicSuffix }.getOrDefault(false) (or treating unparseable as not-allowed). Either way the validation logic itself is an improvement.

Thanks @peterhel for the suggestion, it's definitely better to catch the exception 👍

I've pushed a new commit to catch the exception.

I think we need to catch the exception in some other functions in this file

Thanks for the quick turnaround! Pulled 560440e — builds clean here, and runCatching { … }.getOrDefault(false) is exactly right: an unparseable rpId now falls through to the host check and gets the proper NOT_ALLOWED_ERR. 👍

On catching it elsewhere in the file — agreed; happy to take a pass through the other parse/validation spots and flag any worth the same guard.

Did the pass through the rest of RequestHandling.kt for the same unguarded-throw pattern, and the picture's actually pretty good:

• The other InternetDomainName.from / topDomainOf / Uri.parse call sites (lines 138/142) all live inside isAppIdAllowed, which already wraps its body in a broad try { … } catch (e: Exception) { false } — so a malformed rpId/appId there resolves to a clean NOT_ALLOWED_ERR rather than a crash.
• The Base64.decode(sha256facetId.substring(28), …) in the apk-key-hash branch decodes microG-computed signature digests (from getAllFacetIdCandidates), not RP-supplied data, and the substring(28) is safe because the string is selected by its android:apk-key-hash-sha256: prefix.

So as far as I can see, the line you just fixed was the only RP-reachable crash path — nothing else exposed today.

One forward-looking note rather than an ask: topDomainOf itself throws (InternetDomainName.from → IllegalArgumentException, topDomainUnderRegistrySuffix → IllegalStateException), so any future caller outside a try/catch would reintroduce exactly this issue — might be worth hardening the helper itself (return null on bad input) so callers can't get it wrong. But that's optional and out of scope here.

If you had a specific other function in mind, point me at it and I'll take a closer look. 🙂