Skip to content

Commit 19187e9

Browse files
JulianKastJulianKast
authored
Add a number of NPE safeguards in manager layer (#1842)
* Add null check to prevent NPE in BaseVoiceCommandManager * Add null check to prevent NPE in BaseMenuManager * Add null check to prevent NPE in BaseChoiceSetManager * Add null check to prevent NPE in BaseTextAndGraphicManager * Add null check to prevent NPE in BaseSoftButtonManager * Add null checks to prevent NPE in BaseAlertManager * Add null check to prevent NPE in AudioStreamManager * Add null check to prevent NPE in BaseFileManager * Complete the listener for when we cannot add an operation to the queue in AlertManager * Add callback for when we cannot add operation to Queue in AudioStreamManager * Complete listener if queue is null in BaseChoiceSetManager * add missing return * update ChoiceSetManagerTest
1 parent a37a7ea commit 19187e9

9 files changed

Lines changed: 204 additions & 20 deletions

File tree

‎android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/choiceset/ChoiceSetManagerTests.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ public void tearDown() throws Exception {
113113
assertNull(csm.currentSystemContext);
114114
assertNull(csm.defaultMainWindowCapability);
115115

116-
assertEquals(csm.transactionQueue.getTasksAsList().size(), 0);
116+
assertNull(csm.transactionQueue);
117117

118118
assertFalse(csm.isVROptional);
119119

‎android/sdl_android/src/main/java/com/smartdevicelink/managers/audio/AudioStreamManager.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -437,6 +437,11 @@ public void onDecoderError(Exception e) {
437437
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP) {
438438
decoder = new AudioDecoder(audioSource, context.get(), sdlSampleRate, sdlSampleType, decoderListener);
439439
} else {
440+
if (getTransactionQueue() == null) {
441+
DebugTool.logError(TAG, "Queue is null, cannot push audio source");
442+
finish(completionListener, false);
443+
return;
444+
}
440445
// this BaseAudioDecoder subclass uses methods deprecated with api 21
441446
decoder = new AudioDecoderCompat(getTransactionQueue(), audioSource, context.get(), sdlSampleRate, sdlSampleType, decoderListener);
442447
}

‎base/src/main/java/com/smartdevicelink/managers/file/BaseFileManager.java‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,13 @@ public void onComplete(boolean success, int bytesAvailable, Collection<String> f
159159
}
160160

161161
private void listRemoteFilesWithCompletionListener(final FileManagerCompletionListener completionListener) {
162+
if (transactionQueue == null) {
163+
DebugTool.logError(TAG, "Queue is null, cannot retrieve uploaded files");
164+
if (completionListener != null) {
165+
completionListener.onComplete(false, bytesAvailable, null, "Queue is null, cannot upload files");
166+
}
167+
return;
168+
}
162169
ListFilesOperation operation = new ListFilesOperation(internalInterface, new FileManagerCompletionListener() {
163170
@Override
164171
public void onComplete(boolean success, int bytesAvailable, Collection<String> fileNames, String errorMessage) {
@@ -196,6 +203,13 @@ public void onComplete(boolean success, int bytesAvailable, Collection<String> f
196203
}
197204

198205
private void deleteRemoteFileWithNamePrivate(@NonNull final String fileName, final FileManagerCompletionListener listener) {
206+
if (transactionQueue == null) {
207+
DebugTool.logError(TAG, "Queue is null, cannot delete files");
208+
if (listener != null) {
209+
listener.onComplete(false, bytesAvailable, null, "Queue is null, cannot delete files");
210+
}
211+
return;
212+
}
199213
DeleteFileOperation operation = new DeleteFileOperation(internalInterface, fileName, mutableRemoteFileNames, new FileManagerCompletionListener() {
200214
@Override
201215
public void onComplete(boolean success, int bytesAvailable, Collection<String> fileNames, String errorMessage) {
@@ -406,6 +420,14 @@ private void uploadFilePrivate(@NonNull final SdlFile file, final FileManagerCom
406420
}
407421

408422
private void sdl_uploadFilePrivate(@NonNull final SdlFile file, final FileManagerCompletionListener listener) {
423+
424+
if (transactionQueue == null) {
425+
DebugTool.logError(TAG, "Queue is null, cannot upload files");
426+
if (listener != null) {
427+
listener.onComplete(false, bytesAvailable, null, "Queue is null, cannot upload files");
428+
}
429+
return;
430+
}
409431
final SdlFile fileClone = file.clone();
410432

411433
SdlFileWrapper fileWrapper = new SdlFileWrapper(fileClone, new FileManagerCompletionListener() {

‎base/src/main/java/com/smartdevicelink/managers/screen/BaseAlertManager.java‎

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,17 @@ public void dispose() {
140140
public void presentAlert(AlertView alert, AlertCompletionListener listener) {
141141
if (getState() == ERROR) {
142142
DebugTool.logWarning(TAG, "Alert Manager In Error State");
143+
if (listener != null) {
144+
listener.onComplete(false, null);
145+
}
146+
return;
147+
}
148+
149+
if (transactionQueue == null) {
150+
DebugTool.logError(TAG, "Queue is null, Cannot present Alert.");
151+
if (listener != null) {
152+
listener.onComplete(false, null);
153+
}
143154
return;
144155
}
145156

@@ -149,6 +160,9 @@ public void presentAlert(AlertView alert, AlertCompletionListener listener) {
149160
if (alert.getSoftButtons() != null) {
150161
if (!BaseScreenManager.checkAndAssignButtonIds(alert.getSoftButtons(), BaseScreenManager.ManagerLocation.ALERT_MANAGER)) {
151162
DebugTool.logError(TAG, "Attempted to set soft button objects for Alert, but multiple buttons had the same id.");
163+
if (listener != null) {
164+
listener.onComplete(false, null);
165+
}
152166
return;
153167
}
154168
softButtonObjects.addAll(alert.getSoftButtons());
@@ -216,10 +230,14 @@ protected SoftButtonObject getSoftButtonObjectById(int buttonId) {
216230
private void updateTransactionQueueSuspended() {
217231
if (!isAlertRPCAllowed || currentWindowCapability == null) {
218232
DebugTool.logInfo(TAG, String.format("Suspending the transaction queue. Current permission status is false: %b, window capabilities are null: %b", isAlertRPCAllowed, currentWindowCapability == null));
219-
transactionQueue.pause();
233+
if (transactionQueue != null) {
234+
transactionQueue.pause();
235+
}
220236
} else {
221237
DebugTool.logInfo(TAG, "Starting the transaction queue");
222-
transactionQueue.resume();
238+
if (transactionQueue != null) {
239+
transactionQueue.resume();
240+
}
223241
}
224242
}
225243

@@ -332,6 +350,11 @@ public void onNotified(RPCNotification notification) {
332350

333351
// Updates pending task with new DisplayCapabilities
334352
void updatePendingOperationsWithNewWindowCapability() {
353+
if (transactionQueue == null) {
354+
DebugTool.logError(TAG, "Queue is null, cannot update any Alert operations with new " +
355+
"WindowCapability");
356+
return;
357+
}
335358
for (Task task : transactionQueue.getTasksAsList()) {
336359
if (!(task instanceof PresentAlertOperation)) {
337360
continue;

‎base/src/main/java/com/smartdevicelink/managers/screen/BaseSoftButtonManager.java‎

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,10 @@ public void onCapabilityRetrieved(Object capability) {
158158

159159
// Auto-send an updated Show if we have new capabilities
160160
if (softButtonObjects != null && !softButtonObjects.isEmpty() && softButtonCapabilities != null && !softButtonCapabilitiesEquals(oldSoftButtonCapabilities, softButtonCapabilities)) {
161+
if (transactionQueue == null) {
162+
DebugTool.logError(TAG, "Queue is null, cannot update SoftButtons");
163+
return;
164+
}
161165
SoftButtonReplaceOperation operation = new SoftButtonReplaceOperation(internalInterface, fileManager, softButtonCapabilities, softButtonObjects, getCurrentMainField1(), isDynamicGraphicSupported);
162166
transactionQueue.add(operation, false);
163167
}
@@ -261,10 +265,14 @@ private Queue newTransactionQueue() {
261265
private void updateTransactionQueueSuspended() {
262266
if (softButtonCapabilities == null || HMILevel.HMI_NONE.equals(currentHMILevel)) {
263267
DebugTool.logInfo(TAG, String.format("Suspending the transaction queue. Current HMI level is NONE: %b, soft button capabilities are null: %b", HMILevel.HMI_NONE.equals(currentHMILevel), softButtonCapabilities == null));
264-
transactionQueue.pause();
268+
if (transactionQueue != null) {
269+
transactionQueue.pause();
270+
}
265271
} else {
266272
DebugTool.logInfo(TAG, "Starting the transaction queue");
267-
transactionQueue.resume();
273+
if (transactionQueue != null) {
274+
transactionQueue.resume();
275+
}
268276
}
269277
}
270278

@@ -323,6 +331,10 @@ protected void setSoftButtonObjects(@NonNull List<SoftButtonObject> list) {
323331
batchQueue.clear();
324332
batchQueue.add(operation);
325333
} else {
334+
if (transactionQueue == null) {
335+
DebugTool.logError(TAG, "Queue is null, cannot add SoftButtons");
336+
return;
337+
}
326338
transactionQueue.clear();
327339
transactionQueue.add(operation, false);
328340
}
@@ -357,6 +369,10 @@ private void transitionSoftButton() {
357369
}
358370
batchQueue.add(operation);
359371
} else {
372+
if (transactionQueue == null) {
373+
DebugTool.logError(TAG, "Queue is null, cannot transition SoftButton state");
374+
return;
375+
}
360376
transactionQueue.add(operation, false);
361377
}
362378
}
@@ -397,6 +413,10 @@ protected SoftButtonObject getSoftButtonObjectById(int buttonId) {
397413
* @param batchUpdates Set true if the manager should batch updates together, or false if it should send them as soon as they happen
398414
*/
399415
protected void setBatchUpdates(boolean batchUpdates) {
416+
if (transactionQueue == null) {
417+
DebugTool.logError(TAG, "Queue is null, cannot update SoftButtons");
418+
return;
419+
}
400420
this.batchUpdates = batchUpdates;
401421

402422
if (!this.batchUpdates) {
@@ -425,6 +445,11 @@ protected String getCurrentMainField1() {
425445
* @param currentMainField1 the String that will be set to TextField1 on the current template
426446
*/
427447
protected void setCurrentMainField1(String currentMainField1) {
448+
if (transactionQueue == null) {
449+
DebugTool.logError(TAG, "Queue is null, cannot update MainField1 for SoftButton " +
450+
"operations");
451+
return;
452+
}
428453
this.currentMainField1 = currentMainField1;
429454

430455
for (Task task : transactionQueue.getTasksAsList()) {

‎base/src/main/java/com/smartdevicelink/managers/screen/BaseTextAndGraphicManager.java‎

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -158,10 +158,14 @@ private Queue newTransactionQueue() {
158158
private void updateTransactionQueueSuspended() {
159159
if (defaultMainWindowCapability == null || HMILevel.HMI_NONE.equals(currentHMILevel)) {
160160
DebugTool.logInfo(TAG, String.format("Suspending the transaction queue. Current HMI level is NONE: %b, window capabilities are null: %b", HMILevel.HMI_NONE.equals(currentHMILevel), defaultMainWindowCapability == null));
161-
transactionQueue.pause();
161+
if (transactionQueue != null) {
162+
transactionQueue.pause();
163+
}
162164
} else {
163165
DebugTool.logInfo(TAG, "Starting the transaction queue");
164-
transactionQueue.resume();
166+
if (transactionQueue != null) {
167+
transactionQueue.resume();
168+
}
165169
}
166170
}
167171

@@ -181,6 +185,13 @@ protected void update(CompletionListener listener) {
181185
}
182186

183187
private synchronized void sdlUpdate(final CompletionListener listener) {
188+
if (transactionQueue == null) {
189+
DebugTool.logError(TAG, "Queue is null, cannot perform a text/graphic update");
190+
if (listener != null) {
191+
listener.onComplete(false);
192+
}
193+
return;
194+
}
184195

185196
CurrentScreenDataUpdatedListener currentScreenDataUpdateListener = new CurrentScreenDataUpdatedListener() {
186197
@Override
@@ -225,6 +236,10 @@ void resetFieldsToCurrentScreenData() {
225236

226237
//Updates pending task with current screen data
227238
void updatePendingOperationsWithNewScreenData() {
239+
if (transactionQueue == null) {
240+
DebugTool.logError(TAG, "Queue is null");
241+
return;
242+
}
228243
for (Task task : transactionQueue.getTasksAsList()) {
229244
if (!(task instanceof TextAndGraphicUpdateOperation)) {
230245
continue;
@@ -237,6 +252,10 @@ void updatePendingOperationsWithNewScreenData() {
237252
}
238253

239254
void updatePendingOperationsWithFailedScreenState(TextAndGraphicState errorState) {
255+
if (transactionQueue == null) {
256+
DebugTool.logError(TAG, "Queue is null");
257+
return;
258+
}
240259
for (Task task : transactionQueue.getTasksAsList()) {
241260
if (!(task instanceof TextAndGraphicUpdateOperation)) {
242261
continue;

0 commit comments

Comments
 (0)