Fix NPE in application password creation when site username is null - #22997
Closed
adalpari wants to merge 36 commits into
Closed
Fix NPE in application password creation when site username is null#22997adalpari wants to merge 36 commits into
adalpari wants to merge 36 commits into
Conversation
…#22916) AGP 9's R8 strips the @com.sun.jna.Structure.FieldOrder annotation that wordpress-rs JNA bindings read reflectively at runtime, causing a launch crash in minified release builds. Keep the JNA classes, UniFFI bindings, and the annotation so native struct layouts still resolve.
* Update WordPress `PlayStoreStrings.po` for version 26.8 * Update Jetpack `PlayStoreStrings.po` for version 26.8
* Update editorialized release notes * Update WordPress `PlayStoreStrings.po` for version 26.8 * Update Jetpack `PlayStoreStrings.po` for version 26.8 --------- Co-authored-by: Tony Li <tony.li@automattic.com>
The 26.8 translation sync introduced a doubled backslash (\\") in the Czech confirm_remove_site string. Android reads \\ as an escaped literal backslash, which hides the %s placeholder from lint, so StringFormatMatches fails: ChooseSiteActivity supplies 1 argument but the string is seen as requiring 0. Use a single-backslash escape (\") to match the English source and every other locale.
term_delete_confirmation_message carried the same doubled backslash (\\") as confirm_remove_site, hiding its %1$s placeholder so lint sees 0 args while TermsDataViewActivity supplies 1 (StringFormatMatches). It was the second such error; lint prints only the first failure, so it surfaced once confirm_remove_site was fixed. Also fix the two identical siblings delete_menu_item_confirmation_message and delete_menu_confirmation_message, which share the defect. Verified locally: :WordPress:lintWordpressRelease BUILD SUCCESSFUL, 0 errors.
* Update release notes * Update WordPress `PlayStoreStrings.po` for version 26.8 * Update Jetpack `PlayStoreStrings.po` for version 26.8 --------- Co-authored-by: Tony Li <tony.li@automattic.com>
* Fix false connectivity banner on private Atomic sites Editor capability detection on Atomic sites probes the direct host via unauthenticated REST API discovery (added in #22883). Private Atomic hosts gate anonymous requests, so discovery fails and the My Site "Unable to connect to your site" banner appears even though the site is reachable. Fall back to an authenticated app-password probe against the same host when discovery fails and credentials exist. * Log authenticated direct-host probe failures for diagnosability The authenticated app-password fallback previously returned a silent `false` on a non-success response, and the no-credentials path was also silent. Log both (mirroring the discovery-failure log) so a recurrence of the connectivity banner can be diagnosed from AppLog rather than being indistinguishable from "discovery failed, no creds". * Suppress the connectivity banner while the app password is being minted On first login an Atomic site's application password is minted asynchronously on the My Site screen, so the connectivity capability fetch can race ahead of it and fail purely for lack of credentials — flashing a false "unable to connect" banner that clears on the next refresh. Treat a credential-less fetch as pending rather than a connection failure, mirroring the existing offline suppression; the application-password card already owns the no-credentials state. * Re-detect editor capabilities when an app password is established The connectivity banner's capability fetch and the application-password mint both run on the My Site screen with no ordering, so on first login the fetch loses the race and capability flags aren't detected until the next resume/refresh. Add an app-scoped CredentialsChangedNotifier that both credential-establishment paths (headless mint and interactive login) emit to, and have the connectivity slice re-run detection on it — re-reading a fresh site so it observes the just-persisted credentials. This also closes the asymmetry where only the interactive path announced credential changes globally. * Hoist connectivity banner suppression into a named flag The four-way suppression condition tripped detekt's ComplexCondition threshold once the pending-credentials term was added. Lift it into a named `suppressBanner` flag — clearer intent, and out of the if-condition detekt inspects. No behavior change. * Surface the connectivity banner when the app-password mint fails The pending-credentials suppression also hid the banner when the headless mint *terminally* failed — and the application-password card can't render for private Atomic sites either (its authorize-URL discovery hits the same gated host), so the user was left with a broken site and no signal at all. Track terminal mint failure in the renamed ApplicationPasswordMonitor (was CredentialsChangedNotifier) and suppress only while provisioning is genuinely in flight; once the mint fails, surface the banner. The monitor also wakes capability detection on failure so the banner appears without waiting for the next resume/refresh. * Revert "Surface the connectivity banner when the app-password mint fails" This reverts commit 357fd82. The terminal-failure surfacing called onMintFailed on every non-success, including transient failures, so a transient first-login mint failure latched hasMintFailed and showed the false connectivity banner this PR set out to suppress. Revert to the pending-suppression behavior; the durable single-source-of-truth redesign is tracked in #22942.
Codecov rotated their Keybase username from codecovsecurity to codecovsecops (the new URL serves the same key fingerprint 27034E7FDB850E0BBC2C62FF806BB28AED779869). The old URL now returns a 32-byte 404 body that curl was happily piping into gpg, breaking the import and aborting run-unit-tests.sh under `set -e` before merge_junit_reports could run. Also add `--fail --silent --show-error` so any future move surfaces as a curl failure with a clear message instead of an opaque gpg error. Co-authored-by: Wojtek Zieba <wojtek.zieba@automattic.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…22953) The refresh runs from an async onSettingsUpdated callback that only guards with isAdded(), so mSite can be null when isBlockEditorDefault — a non-null Kotlin param — is invoked, crashing on the generated null check. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…#22954) * Fix Sentry WORDPRESS-ANDROID-3GCQ, WORDPRESS-ANDROID-3GDH: keep recommend models from R8 AGP 9's R8 full mode (introduced in 26.8) was transforming RecommendApiCallsProvider$RecommendTemplateData, causing InstantiationException and ClassCastException when Gson tried to deserialize the /mobile/share-app-link response. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Keep additional Gson-deserialized models from R8 Audit of fromJson<T> call sites surfaced more Kotlin data classes that AGP 9's R8 full mode could class-merge or strip the same way it did to RecommendTemplateData. Adds -keep rules for: - InviteLinksApiCallsProvider.InviteLinksItem - ReaderReadingPreferences (incl. Theme/FontFamily/FontSize enums) - SubfilterListItemMapper.MappedSubfilterListItem - StatsCardsConfiguration + StatsCardType - SubscribersCardsConfiguration + SubscribersCardType - InsightsCardsConfigurationRepository.PersistedConfig + InsightsCardType These have not yet appeared in Sentry for 26.8-rc-5, but the rc-5 user base is small and the affected paths (Reader startup, new stats screens, invite links) are high-traffic enough that they would likely surface in Beta/GA. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
#22952) The Gson-deserialized ApplicationPasswordCreationResponse declared its fields as non-null, but Gson populates objects via reflection and bypasses Kotlin's null checks. When the API returns a 200 response missing the `password` or `uuid` field, those nulls flowed into the non-null ApplicationPasswordCreationPayload constructor, crashing the app (in the minified release build the parameter null-check compiles to a bare getClass() call, producing the reported NPE). Make the response fields nullable and validate them, returning the existing error payload instead of crashing. Applied to both the Jetpack and WP-API clients, which share the response type. Crash report: https://a8c.sentry.io/issues/7522022287/ Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
PR #22885 relaxed the early guard in ApplicationPasswordsManager.getApplicationCredentials from site.isWPCom to site.isWPComSimpleSite, so sites that previously returned NotSupported immediately now flow into getOrFetchUsername. When such a site takes the non-WPCom-REST branch and has a null username, UsernameFetchPayload(site.username) passed null into a non-null String constructor param, triggering Kotlin's null-parameter intrinsic NPE. Guard against a null/empty username and return an error UsernameFetchPayload so getApplicationCredentials degrades to a Failure result instead of crashing. Fixes Sentry 7520323693. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Collaborator
Generated by 🚫 Danger |
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
6 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.
Description
Fixes a regression NPE crash in application password creation.
Sentry: 7520323693
Root cause
PR #22885 ("Allow headless application-password creation on Atomic sites") relaxed the early
guard in
ApplicationPasswordsManager.getApplicationCredentialsfromsite.isWPComtosite.isWPComSimpleSite. Sites that previously returnedNotSupportedimmediately now flowinto
getOrFetchUsername. When such a site takes the non-ORIGIN_WPCOM_RESTbranch and has anull
username, the code didUsernameFetchPayload(site.username)— passingnullinto anon-null
Stringconstructor param, which triggers Kotlin's generated null-parameter intrinsic(
Object.getClass()) and throws the NPE.Fix
Guard against a null/empty username in
getOrFetchUsernameand return an errorUsernameFetchPayloadinstead of crashing.getApplicationCredentialsalready handles theisErrorcase by returningApplicationPasswordCreationResult.Failure, so the feature degradesgracefully (the existing discovery/card path takes over) rather than crashing.
Testing
Added a regression unit test:
given a non-jetpack site with a null username, when we ask for a password, then return Failure. FullApplicationPasswordManagerTestssuite passes.Note
This targets
trunk. The fix also belongs in the 26.8 release, but the remoterelease/26.8branch was deleted, so it should be cherry-picked into the release branch once restored.
🤖 Generated with Claude Code