wordpress-mobile / wordpress-mobile/WordPress-Android
Uncaught NPE in siteResponseToSiteModel when WP.com returns blogging_prompts_settings with null reminders_days/reminders_time
Nobody has claimed this yet.
- Dominant language
- Kotlin
- Stars
- 3.2k
- Forks
- 1.4k
- Avg merge
- 1d 11h
- Merged PRs (30d)
- 69
Description
Summary
SiteRestClient.siteResponseToSiteModel()parsesblogging_prompts_settingsfrom the WP.com sites response and dereferences two sub-fields without a null guard. If the server returnsblogging_prompts_settingsas a non-null object but withreminders_daysorreminders_timenull/absent, the parse throws an uncaughtNullPointerExceptionand the entire site fetch fails.- Both fields are plain unannotated Java fields (Kotlin platform types), so the compiler inserts no caller-side null check — same mechanism as the null-
URLcrash currently live in production (SentryJETPACK-ANDROID-1JAS). - Latent today — no production occurrences yet — but reachable:
blogging_prompts_settingsships underoptions, which is in thefields=allowlist, and there is zero test coverage of this path.
Root cause
In libs/fluxc/src/main/java/org/wordpress/android/fluxc/network/rest/wpcom/site/SiteRestClient.kt, inside siteResponseToSiteModel() (the from.options.blogging_prompts_settings?.let { ... } block, ~L1131–1150). The ?.let guards the parent object being null; it does nothing for the sub-fields:
1. reminders_days — null Map dereference (~L1137–1143) ❌
site.setIsBloggingReminderOnMonday(it.reminders_days["monday"] ?: false)
// ... through Sunday
it.reminders_days["monday"] desugars to it.reminders_days.get("monday"). The ?: false operates on the return value of get, not the receiver — so a null reminders_days map NPEs on the .get call. These seven lines are not inside any try/catch.
reminders_days is declared public Map<String, Boolean> reminders_days; (SiteWPComRestResponse.java:107) — no annotation, no @SerializedName, no custom deserializer. Stock Gson leaves it null when the key is absent or its value is JSON null.
2. reminders_time — null String dereference (~L1144–1149) ❌
try {
site.bloggingReminderHour = it.reminders_time.split(".")[0].toInt()
site.bloggingReminderMinute = it.reminders_time.split(".")[1].toInt()
} catch (ex: NumberFormatException) {
AppLog.e(API, "Received malformed blogging reminder time: " + ex.message)
}
it.reminders_time.split(".") throws NPE on a null reminders_time. The surrounding try catches only NumberFormatException — NullPointerException is not a subclass of it (NFE → IllegalArgumentException; NPE → RuntimeException), so the NPE escapes the catch. reminders_time is public String reminders_time; (SiteWPComRestResponse.java:108), same platform-type/Gson story.
The crash propagates uncaught out of siteResponseToSiteModel() to its callers (fetchSite, fetchSites, and the site-feature path).
Why it can happen
The "the server always sends a complete blogging_prompts_settings object" assumption is unverified and unenforced — no @NonNull, no test, no fixture. We have direct, recent production proof that this exact endpoint violates field-presence assumptions: site.url = from.URL reads an identically-unannotated field and just started returning null in production (Sentry JETPACK-ANDROID-1JAS, ~1,300 crashes / ~180 users in the first ~7.5h). The method itself already distrusts this object's contents (the NumberFormatException catch on reminders_time) — it just doesn't guard the fields' presence. /me/sites/ and /sites/$site/ are also documented in-method as returning different shapes for the same fields.
Evidence
Adversarially validated by three independent reviewers; two reproduced the exact exceptions with a standalone kotlinc repro of the platform-type + deref pattern:
reminders_daysnull →java.lang.NullPointerException: Cannot invoke "java.util.Map.get(Object)" because "it.reminders_days" is nullreminders_timenull →java.lang.NullPointerException: reminders_time must not be null(KotlinIntrinsics.checkNotNullExpressionValueemitted beforesplit)
Bytecode confirmed the ?: false Elvis sits on the get result, after the null receiver has already been invoked.
Fix
Null-safe both dereferences (the surrounding code already treats this object as untrusted):
// reminders_days — all seven day lines:
site.setIsBloggingReminderOnMonday(it.reminders_days?.get("monday") ?: false)
// reminders_time — safe-call before split (or broaden the catch to cover NPE/IndexOutOfBounds):
it.reminders_time?.split(".")?.let { parts ->
try {
site.bloggingReminderHour = parts[0].toInt()
site.bloggingReminderMinute = parts[1].toInt()
} catch (ex: NumberFormatException) {
AppLog.e(API, "Received malformed blogging reminder time: " + ex.message)
}
}
A regression test that feeds siteResponseToSiteModel a blogging_prompts_settings with each sub-field null would lock this in — the path currently has no coverage.
Related
- Live null-
URLincident (same root pattern, different field): SentryJETPACK-ANDROID-1JAS. TheURLcase is being addressed server-side (it's the only field that crashes thejava.net.URIparser); these two are the remaining client-side null-deref siblings in the same parse method.
Found while triaging the null-URL crash.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in libs/fluxc/src/main/java/org/wordpress/android/fluxc/network/rest/wpcom/site/SiteRestClient.kt at siteResponseToSiteModel(), especially the blogging_prompts_settings block. Review the null handling for reminders_days and reminders_time, then add regression coverage that supplies each sub-field as null and confirms site parsing completes without an exception. Use the existing SiteWPComRestResponse.java field declarations to understand the response shape.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- java, kotlin
- Domain
- api, mobile-dev
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 76/100