[Static Analysis][Dereference null return value] In function ReadingListHelper::fetchContent

RESOLVED FIXED in Firefox 47

Status

()

defect
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: andi, Assigned: andi)

Tracking

(Blocks 1 bug, {coverity})

unspecified
Firefox 47
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox47 fixed)

Details

(Whiteboard: CID 123632)

Attachments

(1 attachment)

The Static Analysis tool Coverity added that a variable |c| can cause a null pointer dereference in the following context: 

>>                final Cursor c = readingListAccessor.getReadingListUnfetched(context.getContentResolver());
>>                try {
>>                    while (c.moveToNext()) {
>>                        JSONObject json = new JSONObject();
>>                        try {
>>                            json.put("id", c.getInt(c.getColumnIndexOrThrow(ReadingListItems._ID)));
>>                            json.put("url", c.getString(c.getColumnIndexOrThrow(ReadingListItems.URL)));
>>                            GeckoAppShell.sendEventToGecko(
>>                                GeckoEvent.createBroadcastEvent("Reader:FetchContent", json.toString()));
>>                        } catch (JSONException e) {

As |readingListAccessor| is an instance of LocalReadingListAccessor function getReadingListUnfetched calls a query that it's return is NullAble:

>>    public final @Nullable Cursor query(@NonNull Uri uri, @Nullable String[] projection,
>>            @Nullable String selection, @Nullable String[] selectionArgs,
>>            @Nullable String sortOrder) {
>>        return query(uri, projection, selection, selectionArgs, sortOrder, null);
>>    }

thus |c| can be null
Comment on attachment 8724728 [details]
MozReview Request: Bug 1252082 - prevent null pointer dereference on |c|. r?sebastian

https://reviewboard.mozilla.org/r/37127/#review33697

::: mobile/android/base/java/org/mozilla/gecko/ReadingListHelper.java:254
(Diff revision 1)
> +                    if ( c != null ) {

NIT: I'd prefer an early return here (cursor == null) instead of wrapping the whole block. Also: We usually do not add spaces around the expression: if (cursor != null).
Attachment #8724728 - Flags: review?(s.kaspari) → review+
Comment on attachment 8724728 [details]
MozReview Request: Bug 1252082 - prevent null pointer dereference on |c|. r?sebastian

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/37127/diff/1-2/
https://hg.mozilla.org/mozilla-central/rev/1846e981637e
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 47
You need to log in before you can comment on or make changes to this bug.