New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 627503 link

Starred by 2 users

Issue metadata

Status: Assigned
Owner:
Cc:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug



Sign in to add a comment

Nullpointer crash in CustomTabNavigationDelegate.startActivityIfNeeded

Project Member Reported by gsennton@chromium.org, Jul 12 2016

Issue description

There are about 20k crashes with the stack

java.lang.NullPointerException
at java.net.URI.parseURI	(URI.java:353 )
at java.net.URI.<init>	(URI.java:204 )
at org.chromium.chrome.browser.util.UrlUtilities.isAcceptedScheme	(UrlUtilities.java:69 )
at org.chromium.chrome.browser.customtabs.CustomTabDelegateFactory$CustomTabNavigationDelegate.startActivityIfNeeded	(CustomTabDelegateFactory.java:58 )
at org.chromium.chrome.browser.externalnav.ExternalNavigationHandler.shouldOverrideUrlLoadingInternal	(ExternalNavigationHandler.java:446 )
at org.chromium.chrome.browser.externalnav.ExternalNavigationHandler.shouldOverrideUrlLoading	(ExternalNavigationHandler.java:116 )
at org.chromium.chrome.browser.tab.InterceptNavigationDelegateImpl.shouldIgnoreNavigation	(InterceptNavigationDelegateImpl.java:112 )
at org.chromium.base.SystemMessageHandler.nativeDoRunLoopOnce	(Native Method )
at org.chromium.base.SystemMessageHandler.handleMessage	(SystemMessageHandler.java:39 )
at android.os.Handler.dispatchMessage	(Handler.java:102 )
at android.os.Looper.loop	(Looper.java:136 )
at android.app.ActivityThread.main	(ActivityThread.java:5052 )
at java.lang.reflect.Method.invokeNative	(Native Method )
at java.lang.reflect.Method.invoke	(Method.java:515 )
at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run	(ZygoteInit.java:793 )
at com.android.internal.os.ZygoteInit.main	(ZygoteInit.java:609 )
at dalvik.system.NativeStart.main	(Native Method )


link to crash.corp (see the top 3 magic signatures):

https://crash.corp.google.com/browse?q=stable_signature%3D%27java.lang.NullPointerException%20at%20java.net.URI.parseURI-ddf4aede%27&ignore_case=false&enable_rewrite=true&omit_field_name=&omit_field_value=&omit_field_opt=%3D#samplereports:5,productversion,magicsignature


Given that crbug.com/580661 fixed a null-pointer exception where intent.getData() was null for a similar code path as the above stack it is reasonable to believe that intent.getData() can be null in this case as well, and this is what causes the crash?


Not sure if UrlUtilities.isAcceptedScheme(String uri) should null-check the String passed to it, or whether CustomTabDelegateFactory.startActivityIfNeeded should null-check intent.getDataString() (or both).

Adding yusufo@ who wrote the CustomTabDelegateFactory code and rmcilroy@ who upstreamed the UrlUtilities class.
 

Comment 1 by yus...@chromium.org, Jul 12 2016

I think it would be better to do it inside the delegate factory and return false early on if there is no data string.

But looking at the crash stacks, do we see this on m52 and later?

Comment 3 by yus...@chromium.org, Jul 12 2016

Thanks.

Fix uploaded here : https://codereview.chromium.org/2147543003/
Project Member

Comment 4 by bugdroid1@chromium.org, Aug 9 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4128ae6b96c70dc721b4ae5ce9d7707169e3b348

commit 4128ae6b96c70dc721b4ae5ce9d7707169e3b348
Author: yusufo <yusufo@chromium.org>
Date: Tue Aug 09 19:44:33 2016

Check if there is a url before checking to launch activity inside CCT

In some cases, on a user click in a page, we might get an intent with
a null data string. In these cases we are crashing while checking
whether the uri has an accepted scheme.

This adds an early return to avoid the crash.

BUG=627503

Review-Url: https://codereview.chromium.org/2147543003
Cr-Commit-Position: refs/heads/master@{#410769}

[modify] https://crrev.com/4128ae6b96c70dc721b4ae5ce9d7707169e3b348/chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabDelegateFactory.java

Sign in to add a comment