1031210 - Crash - java.lang.StringIndexOutOfBoundsException: length=146; regionStart=0; regionLength=150 at org.mozilla.gecko.mozglue.GeckoLoader.nativeRun(Native Method) when starting Gecko with a URL like http://www.iciba.com/德国

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect, P5)

Description

[sulliwane]

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:30.0) Gecko/20100101 Firefox/30.0 (Beta/Release)
Build ID: 20140605174243

Steps to reproduce:

From my chinese dictionary app (Hanping) I click on a link to open an URL in a website (shortcut to query a word search in an online dictionary), then choose to open with Firefox browser. If firefox was not previously running (after a phone reboot, or after killing firefox app) it makes firefox crash.


Actual results:

Firefox crash (black screen for 1 second).


Expected results:

Firefox should have open the requested website looking for the URL specified by the other app (dictionary app in my case).

Comment 1

[sulliwane]

Here is the error log :

06-27 15:36:47.299: I/PackageManager(749):   Action: "android.intent.action.VIEW"
06-27 15:36:47.299: I/PackageManager(749):   Category: "android.intent.category.DEFAULT"
06-27 15:36:47.299: I/PackageManager(749):   Scheme: "http"
06-27 15:36:47.299: I/PackageManager(749): Adding preferred activity ComponentInfo{org.mozilla.firefox/org.mozilla.firefox.App} for user 0 :
06-27 15:36:47.319: I/ActivityManager(749): START u0 {act=android.intent.action.VIEW dat=http://www.iciba.com/德国 flg=0x3000000 cmp=org.mozilla.firefox/.App} from pid 20432
06-27 15:36:47.459: W/GeckoProfile(21704): Requested profile directory missing.
06-27 15:36:47.489: D/GeckoScreenOrientation(21704): updating to new orientation PORTRAIT_PRIMARY
06-27 15:36:47.599: D/GeckoLocales(21704): Calling setContextGetter: org.mozilla.firefox.App@42c1f0d0
06-27 15:36:47.609: D/GeckoSessInfo(21704): Building SessionInformation from prefs: 1403854568297, 0, false, false
06-27 15:36:47.609: D/GeckoProfile(21704): Found profile dir.
06-27 15:36:47.609: I/GeckoApp(21704): Creating HealthRecorder.
06-27 15:36:47.609: D/GeckoApp(21704): OS locale is en_GB, app locale is null
06-27 15:36:47.609: D/GeckoHealthRec(21704): Initializing. Dispatcher is org.mozilla.gecko.EventDispatcher@42c022e0
06-27 15:36:47.609: D/GeckoHealthRec(21704): Initializing profile cache.
06-27 15:36:47.609: D/GeckoHardwareUtils(21704): System memory: 1855MB.
06-27 15:36:47.609: I/GeckoAnnounce(21704): firefox :: GeckoProfileInfo :: Restoring ProfileInformationCache from file.
06-27 15:36:47.619: D/GeckoHealthRec(21704): Successfully restored state. Initializing storage.
06-27 15:36:47.619: D/GeckoHealthRec(21704): Done initializing profile cache. Beginning storage init.
06-27 15:36:47.629: I/GeckoAnnounce(21704): firefox :: HealthReportStorage :: Initializing measurement org.mozilla.appSessions to 4 (current 4)
06-27 15:36:47.629: I/GeckoAnnounce(21704): firefox :: HealthReportStorage :: Measurement org.mozilla.appSessions already at v4
06-27 15:36:47.629: I/GeckoAnnounce(21704): firefox :: HealthReportStorage :: Initializing measurement org.mozilla.searches.counts to 5 (current 5)
06-27 15:36:47.629: I/GeckoAnnounce(21704): firefox :: HealthReportStorage :: Measurement org.mozilla.searches.counts already at v5
06-27 15:36:47.629: D/GeckoHealthRec(21704): Ensuring environment.
06-27 15:36:47.629: W/GeckoEventDispatcher(21704): unregisterEventListener: event 'Prompt:ShowTop' has no listeners
06-27 15:36:47.639: D/GeckoHealthRec(21704): Finishing init.
06-27 15:36:47.639: D/GeckoHealthRec(21704): Checking for orphan session.
06-27 15:36:47.659: D/GeckoApp(21704): onConfigurationChanged: en_GB
06-27 15:36:47.659: V/GeckoHealthRec(21704): Recorded session entry for env 1, current is 1
06-27 15:36:47.659: D/GeckoSessInfo(21704): Recording start of session: 1403854607641
06-27 15:36:47.739: D/OpenGLRenderer(21704): Enabling debug mode 0
06-27 15:36:47.749: W/GeckoGLController(21704): GLController::serverSurfaceChanged(1080, 1701)
06-27 15:36:47.889: W/ActivityThread(21704): ClassLoader.loadClass: The class loader returned by Thread.getContextClassLoader() may fail for processes that host multiple applications. You should explicitly specify a context class loader. For example: Thread.setContextClassLoader(getClass().getClassLoader());
06-27 15:36:47.899: I/GeckoAxis(21704): Prefs: 0.85,0.97,10.0,0.1,0.04,0.3,0.5
06-27 15:36:47.899: I/ActivityManager(749): Displayed org.mozilla.firefox/.App: +479ms
06-27 15:36:47.909: D/GeckoLoader(21704): Gecko environment env0: null
06-27 15:36:47.919: E/GeckoLibLoad(21704): Load sqlite start
06-27 15:36:47.919: E/GeckoLinker(21704): /data/app/org.mozilla.firefox-1.apk!/assets/libnss3.so: Warning: unhandled flags #8 not handled
06-27 15:36:47.919: W/GeckoGLController(21704): GLController::updateCompositor with mCompositorCreated=false
06-27 15:36:47.919: W/GeckoGLController(21704): done GLController::updateCompositor
06-27 15:36:47.949: E/GeckoLibLoad(21704): Load sqlite done
06-27 15:36:47.949: E/GeckoLibLoad(21704): Load nss start
06-27 15:36:47.949: E/GeckoLibLoad(21704): Load nss done
06-27 15:36:47.949: E/GeckoLinker(21704): /data/app/org.mozilla.firefox-1.apk!/assets/libxul.so: Warning: unhandled flags #8 not handled
06-27 15:36:47.949: E/GeckoLinker(21704): /data/app/org.mozilla.firefox-1.apk!/assets/libmozalloc.so: Warning: unhandled flags #8 not handled
06-27 15:36:47.969: E/GeckoLinker(21704): /data/app/org.mozilla.firefox-1.apk!/assets/libxul.so: Warning: relocation to NULL @0x016c42e8
06-27 15:36:47.969: E/GeckoLinker(21704): /data/app/org.mozilla.firefox-1.apk!/assets/libxul.so: Warning: relocation to NULL @0x016c5410 for symbol "__cxa_begin_cleanup"
06-27 15:36:47.969: E/GeckoLinker(21704): /data/app/org.mozilla.firefox-1.apk!/assets/libxul.so: Warning: relocation to NULL @0x016c5414 for symbol "__cxa_type_match"
06-27 15:36:48.009: W/GeckoGLController(21704): GLController::serverSurfaceChanged(1080, 1557)
06-27 15:36:48.039: D/GeckoToolbar(21704): onTabChanged: SELECTED
06-27 15:36:48.049: D/GeckoToolbarDisplayLayout(21704): updateFavicon(null)
06-27 15:36:48.049: I/GeckoToolbarDisplayLayout(21704): zerdatime 14675543 - Throbber start
06-27 15:36:48.049: D/GeckoBrowserApp(21704): BrowserApp.onTabChanged: 0: SELECTED
06-27 15:36:48.049: D/GeckoToolbar(21704): onTabChanged: RESTORED
06-27 15:36:48.049: W/GeckoGLController(21704): GLController::updateCompositor with mCompositorCreated=false
06-27 15:36:48.049: W/GeckoGLController(21704): done GLController::updateCompositor
06-27 15:36:48.099: E/GeckoLibLoad(21704): Loaded libs in 151ms total, 50ms(120ms) user, 30ms(40ms) system, 0(0) faults
06-27 15:36:48.099: W/GeckoThread(21704): zerdatime 14675597 - runGecko
06-27 15:36:48.099: I/GeckoThread(21704): RunGecko - args =  -P default
06-27 15:36:48.109: D/GeckoAppShell(21704): GeckoLoader.nativeRun /data/app/org.mozilla.firefox-1.apk -greomni /data/app/org.mozilla.firefox-1.apk  -P default -url http://www.iciba.com/德国 -width 1080 -height 1776
06-27 15:36:48.109: E/Profiler(21704): BEGIN mozilla_sampler_init
06-27 15:36:48.109: E/Profiler(21704): BPUnw: [1 total] thread_register_for_profiling(me=0x77f3f008, stacktop=0x78c61972)
06-27 15:36:48.109: E/Profiler(21704): SPS:
06-27 15:36:48.109: E/Profiler(21704): SPS: Unwind mode       = pseudo
06-27 15:36:48.109: E/Profiler(21704): SPS: Sampling interval = 0 ms (zero means "platform default")
06-27 15:36:48.109: E/Profiler(21704): SPS: Entry store size  = 0 (zero means "platform default")
06-27 15:36:48.109: E/Profiler(21704): SPS: UnwindStackScan   = 0 (max dubious frames per unwind).
06-27 15:36:48.109: E/Profiler(21704): SPS: Use env var MOZ_PROFILER_MODE=help for further information.
06-27 15:36:48.109: E/Profiler(21704): SPS:
06-27 15:36:48.109: E/Profiler(21704): Registering start signal
06-27 15:36:48.119: E/Profiler(21704): BPUnw: [2 total] thread_register_for_profiling(me=0x74eccc80, stacktop=0x7e772caf)
06-27 15:36:48.139: E/GeckoConsole(21704): Could not read chrome manifest 'file:///data/data/org.mozilla.firefox/chrome.manifest'.
06-27 15:36:48.139: I/Gecko:MemoryInfoDumper(21704): Fifo watcher disabled via pref.
06-27 15:36:48.319: E/Profiler(21704): BPUnw: [1 total] thread_unregister_for_profiling(me=0x74eccc80) 
06-27 15:36:48.319: I/fennec(21704): XRE_main returned 1
06-27 15:36:48.319: E/GeckoAppShell(21704): >>> REPORTING UNCAUGHT EXCEPTION FROM THREAD 929 ("Gecko")
06-27 15:36:48.319: E/GeckoAppShell(21704): java.lang.StringIndexOutOfBoundsException: length=146; regionStart=0; regionLength=150
06-27 15:36:48.319: E/GeckoAppShell(21704): at org.mozilla.gecko.mozglue.GeckoLoader.nativeRun(Native Method)
06-27 15:36:48.319: E/GeckoAppShell(21704): at org.mozilla.gecko.GeckoAppShell.runGecko(GeckoAppShell.java:346)
06-27 15:36:48.319: E/GeckoAppShell(21704): at org.mozilla.gecko.GeckoThread.run(GeckoThread.java:178)
06-27 15:36:48.319: E/GeckoAppShell(21704): Main thread stack:
06-27 15:36:48.319: E/GeckoAppShell(21704): android.os.MessageQueue.nativePollOnce(Native Method)
06-27 15:36:48.319: E/GeckoAppShell(21704): android.os.MessageQueue.next(MessageQueue.java:138)
06-27 15:36:48.319: E/GeckoAppShell(21704): android.os.Looper.loop(Looper.java:123)
06-27 15:36:48.319: E/GeckoAppShell(21704): android.app.ActivityThread.main(ActivityThread.java:5001)
06-27 15:36:48.319: E/GeckoAppShell(21704): java.lang.reflect.Method.invokeNative(Native Method)
06-27 15:36:48.319: E/GeckoAppShell(21704): java.lang.reflect.Method.invoke(Method.java:515)
06-27 15:36:48.319: E/GeckoAppShell(21704): com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:785)
06-27 15:36:48.319: E/GeckoAppShell(21704): com.android.internal.os.ZygoteInit.main(ZygoteInit.java:601)
06-27 15:36:48.319: E/Gecko(21704): mozalloc_abort: Redirecting call to abort() to mozalloc_abort
06-27 15:36:48.319: A/libc(21704): Fatal signal 11 (SIGSEGV) at 0x00000000 (code=1), thread 21797 (Gecko)

Comment 2

[Aaron Train [:aaronmt]]

Thanks for the report. The key here is the URL fed to Gecko: 

GeckoLoader.nativeRun /data/app/org.mozilla.firefox-1.apk -greomni /data/app/org.mozilla.firefox-1.apk  -P default -url http://www.iciba.com/德国 -width 1080 -height 1776

This crashes the browser:

adb shell am start -a android.intent.action.VIEW -n org.mozilla.fennec/.App -d "http://www.iciba.com/德国"

Comment 3

[Aaron Train [:aaronmt]]

Simplified just by launching Gecko with a UTF-8 Unicode character, e.g, org.mozilla.fennec/.App -d א

Comment 4

[Brad Lassey [:blassey] (use needinfo?)]

filter on [mass-p5]

Comment 5

[David Weir (satdav)]

https://crash-stats.mozilla.com/report/index/9e7a7f60-5dd1-4bc4-895c-de9022150325

Comment 6

[Richard Newman [:rnewman]]

"crashed when opened a Google plus page"

Great, so no users affected by that. *rimshot*

Comment 7

[Richard Newman [:rnewman]]

  // XXX: java doesn't give us true UTF8, we should figure out something
  // better to do here
  int len = jenv->GetStringUTFLength(jargs);
  // GeckoStart needs to write in the args buffer, so we need a copy.
  char *args = (char *) malloc(len + 1);
  jenv->GetStringUTFRegion(jargs, 0, len, args);


That looks pretty suspect.

Comment 8

[David Weir (satdav)]

I managed to get this crash on etherpad

Comment 10

[Kevin Brosnan [Ex-Mozilla]]

bug 1331547 comment 0 has some new steps.

Comment 11

[Mike Connor [:mconnor]]

https://crash-stats.mozilla.com/report/index/7f52c73d-8026-4ae7-a8b1-4aa4e2170208#tab-details

I'm not sure this is a true dupe, but I'll let snorp make that call.

The intent opening the browser in the dupe is passing in "http://".  Leaving aside whether that is a useful behaviour, we're stripping the scheme and ending up with an empty string.  There's probably multiple places we should guard against this, but at the least we should ensure that stripScheme [1] is a little smarter...

[1] https://dxr.mozilla.org/mozilla-central/source/mobile/android/geckoview/src/main/java/org/mozilla/gecko/util/StringUtils.java#108

Comment 12

[Mike Kaply [:mkaply]]

Easy way to recreate this crash using adb. Close the browser.

db shell am start -a android.intent.action.VIEW -c android.intent.category.default -d http:// -n org.mozilla.firefox/.App

Comment 13

[Mike Kaply [:mkaply]]

Neither of the first two original scenarios crash the browser anymore (http://www.iciba.com/德国 or א).

Comment 14

[Mike Kaply [:mkaply]]

Attachment: Better assumptions about URLs

The core problem here is that we always assumed that trailing slash should be removed. In the http:// case, that caused bad things to happen.

I reworked the code to remove http/https at the beginning and then handle the / at the end in the new URL.

Comment 15

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

plus it since there's patch under review.

Comment 16

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8838770 [details] [diff] [review]
Better assumptions about URLs

Review of attachment 8838770 [details] [diff] [review]:
-----------------------------------------------------------------

This whole function is just weird. It really seems like we should not be parsing the URI ourselves, and instead using something like android.net.Uri. A quick glance there indicates that it may be a little cumbersome to return the string representation that only differs by excluding the Uri, but I think it should generally be good enough. Ugh. It's all kinda terrible.

Comment 17

[Mike Kaply [:mkaply]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d5bc4f182ec309b2d9577b5d7a0414d9d4ed7c0b
Bug 1031210 - Don't assume URL format to prevent crash. r=snorp

Comment 18

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/d5bc4f182ec3

Comment 19

[Mike Kaply [:mkaply]]

Comment on attachment 8838770 [details] [diff] [review]
Better assumptions about URLs

Approval Request Comment
[Feature/Bug causing the regression]: Fix crash bug.
[User impact if declined]: Crash if URL "http://" is opened programmatically somehow (this happens with our partner).
[Is this code covered by automated tests?]: No
[Has the fix been verified in Nightly?]: Not yet.
[Needs manual test from QE? If yes, steps to reproduce]: run the command:

adb shell am start -a android.intent.action.VIEW -c android.intent.category.default -d http:// -n org.mozilla.firefox/.App
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: Low
[Why is the change risky/not risky?]: Changes existing string code to be more robust
[String changes made/needed]:

I realize this is late in the cycle, but this was found by our partner and can be easily recreated by them (and their users) on the device.

Comment 20

[Julien Cristau [:jcristau] (back August 18)]

Any chance of some additions to mobile/android/tests/background/junit4/src/org/mozilla/gecko/util/TestStringUtils.java?

Comment 21

[Mike Kaply [:mkaply]]

Yes, I will take a look at writing those today.

Comment 22

[Mike Kaply [:mkaply]]

Attachment: Unit tests for stripScheme function

Complete tests for stripScheme.

I did run these tests on the old code and it did fail:

java.lang.StringIndexOutOfBoundsException: String index out of range: -1

Comment 23

[Gerry Chang [:gchang]]

Hi Brindusa, could you help find someone to verify if this issue was fixed as expected on a latest Nightly build? Thanks!

Comment 24

[Brindusa Tot, Desktop QA]

I will move this to Fennec team, as I am not part of it. Ioana, could you or someone from you team, take a look on this? Thanks!

Comment 25

[Ioana Chiorean]

Thanks Brindusa! Bogdan will work on it.

Comment 26

[Bogdan Surd, QA [:bsurd, NI]]

Devices:
 - HTC Desire 820 (Android 6.0);
 - Samsung Galaxy Note 4 (Android 5.1.1).

 Hello, I have verified this issue, it would seem that the problem is fixed, I didn't encounter any problems. Marking as Verified.

Notes:
 - On the HTC if the user manually entered the characters in the URL instead of the Unicode text format the page would not load anymore.
 - No such problems were encountered on the Samsung.

Comment 27

[Mike Kaply [:mkaply]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4ee8c67b75bc1e300ffa5376198b1b991d0928e7
Bug 1031210 - Add junit tests for StringUtils.stripScheme. r=snorp

Comment 28

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/4ee8c67b75bc

Comment 29

[Julien Cristau [:jcristau] (back August 18)]

Comment on attachment 8838770 [details] [diff] [review]
Better assumptions about URLs

fix a fennec crash affecting a partner, aurora53+, beta/release 52+

Comment 30

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/2a487fe0f3d8a0490bb2d6a3bed75b803d062e06
https://hg.mozilla.org/releases/mozilla-aurora/rev/984adcec5b2e720c5f1257f136eaa5dfb3a21fd8

Comment 31

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/60b304c27670
https://hg.mozilla.org/releases/mozilla-beta/rev/7ea82d65f876

Comment 32

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-release/rev/60b304c27670
https://hg.mozilla.org/releases/mozilla-release/rev/7ea82d65f876

Comment 33

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr52/rev/60b304c27670
https://hg.mozilla.org/releases/mozilla-esr52/rev/7ea82d65f876

Comment 34

[u549602]

Verified as fixed on both latest Aurora 53.0a2 (03-03-2017) and 52.0b11 on a Samsung Galaxy S6 Edge (Android 6.0) and Samsung Galaxy Tab Active (Android 5.1.1)