1316008 - [findbugs] [Dm] Found reliance on default encoding

Status: RESOLVED FIXED

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

Description

[Sebastian Kaspari (:sebastian; :pocmo)]

* Found reliance on default encoding in org.mozilla.gecko.ANRReporter.getTracesFile(): new java.io.InputStreamReader(InputStream)

* Found reliance on default encoding in org.mozilla.gecko.ANRReporter.isGeckoTraces(String, File): new java.io.FileReader(File)

* Found reliance on default encoding in org.mozilla.gecko.background.common.log.Logger.startLoggingToConsole(): new java.io.PrintWriter(OutputStream, boolean)

* Found reliance on default encoding in org.mozilla.gecko.BrowserApp$34.onIconResponse(IconResponse): new String(byte[])

* Found reliance on default encoding in org.mozilla.gecko.BrowserApp$34.onIconResponse(IconResponse): String.getBytes()

* Found reliance on default encoding in org.mozilla.gecko.browserid.JSONWebTokenUtils.parseAssertion(String): new String(byte[])

* Found reliance on default encoding in org.mozilla.gecko.browserid.JSONWebTokenUtils.parseCertificate(String): new String(byte[])

* Found reliance on default encoding in org.mozilla.gecko.db.LoginsProvider.decrypt(String): new String(byte[])

* Found reliance on default encoding in org.mozilla.gecko.fxa.authenticator.AccountPickler.writeToDisk(Context, String, ExtendedJSONObject): new java.io.PrintStream(OutputStream)

* Found reliance on default encoding in org.mozilla.gecko.home.BrowserSearch.getDomains(): new java.io.InputStreamReader(InputStream)

* Found reliance on default encoding in org.mozilla.gecko.media.GeckoMediaDrmBridgeV21.closeSession(int, String): String.getBytes()
	
* Found reliance on default encoding in org.mozilla.gecko.media.GeckoMediaDrmBridgeV21.createSession(int, int, String, byte[]): new String(byte[])
	
* Found reliance on default encoding in org.mozilla.gecko.media.GeckoMediaDrmBridgeV21.ensureMediaCryptoCreated(): new String(byte[])
	
* Found reliance on default encoding in org.mozilla.gecko.media.GeckoMediaDrmBridgeV21.updateSession(int, String, byte[]): String.getBytes()
	
* Found reliance on default encoding in org.mozilla.gecko.media.GeckoMediaDrmBridgeV21$MediaDrmListener.onEvent(MediaDrm, byte[], int, int, byte[]): new String(byte[])
	
* Found reliance on default encoding in org.mozilla.gecko.media.GeckoMediaDrmBridgeV21$PostRequestTask.doInBackground(Void[]): new java.io.InputStreamReader(InputStream)
	
* Found reliance on default encoding in org.mozilla.gecko.media.GeckoMediaDrmBridgeV21$PostRequestTask.doInBackground(Void[]): new String(byte[])
	
* Found reliance on default encoding in org.mozilla.gecko.media.GeckoMediaDrmBridgeV21$PostRequestTask.doInBackground(Void[]): String.getBytes()

* Found reliance on default encoding in org.mozilla.gecko.search.SearchEngineManager.fetchCountryCode(): String.getBytes()
	
* Found reliance on default encoding in org.mozilla.gecko.search.SearchEngineManager.getHttpResponse(HttpURLConnection): new java.util.Scanner(InputStream)

* Found reliance on default encoding in org.mozilla.gecko.SuggestClient.convertStreamToString(InputStream): new java.util.Scanner(InputStream)
	
* Found reliance on default encoding in org.mozilla.gecko.sync.CollectionKeys.keyBundleToArray(KeyBundle): new String(byte[])
	
* Found reliance on default encoding in new org.mozilla.gecko.sync.crypto.KeyBundle(String, String): String.getBytes()
	
* Found reliance on default encoding in org.mozilla.gecko.sync.CryptoRecord.encrypt(): new String(byte[])

* Found reliance on default encoding in org.mozilla.gecko.sync.net.MozResponse.body(): new java.io.InputStreamReader(InputStream)
	
* Found reliance on default encoding in org.mozilla.gecko.sync.net.SyncStorageCollectionRequest$SyncCollectionResourceDelegate.handleHttpResponse(HttpResponse): new java.io.InputStreamReader(InputStream)
	
* Found reliance on default encoding in org.mozilla.gecko.sync.Utils.generateGuid(): new String(byte[])
	
* Found reliance on default encoding in org.mozilla.gecko.sync.Utils.readFile(Context, String): new java.io.InputStreamReader(InputStream)

* Found reliance on default encoding in org.mozilla.mozstumbler.service.stumblerthread.datahandling.DataStorageManager.getFirstBatch(): String.getBytes()
	
* Found reliance on default encoding in org.mozilla.mozstumbler.service.stumblerthread.datahandling.DataStorageManager.getQueuedCounts(): String.getBytes()
	
* Found reliance on default encoding in org.mozilla.mozstumbler.service.stumblerthread.datahandling.DataStorageManager.saveCurrentReportsToDisk(): String.getBytes()

* Found reliance on default encoding in org.mozilla.mozstumbler.service.utils.Zipper.unzipData(byte[]): new java.io.InputStreamReader(InputStream)

Comment 1

[Sebastian Kaspari (:sebastian; :pocmo)]

"Found a call to a method which will perform a byte to String (or String to byte) conversion, and will assume that the default platform encoding is suitable. This will cause the application behaviour to vary between platforms. Use an alternative API and specify a charset name or Charset object explicitly."

Comment 2

[Sebastian Kaspari (:sebastian; :pocmo)]

To start, set up a build environment - you can see the instructions here:
https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions/Simple_Firefox_for_Android_build

Then, you'll need to upload a patch - see:
http://mozilla-version-control-tools.readthedocs.io/en/latest/mozreview/commits.html

If you need any help, you can reply to this bug, or feel free to message me on IRC - my nick is "sebastian" and you can find me and other helpful folks in #mobile. If you need IRC setup instructions, see https://wiki.mozilla.org/IRC

Comment 3

[Adrian Zatreanu]

Attachment: Bug 1316008: Use explicit charset encoding

Review commit: https://reviewboard.mozilla.org/r/96548/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/96548/

Comment 4

[Sebastian Kaspari (:sebastian; :pocmo)]

After a quick glimpse I think this looks good, but Nevin, can you review this patch? I've to distribute some of my requests. Too much going on. :)

Comment 5

[Nevin Chen(Not active on Bugzilla)]

Comment on attachment 8815685 [details]
Bug 1316008: Use explicit charset encoding

LGTM. Thanks!

Comment 6

[Nevin Chen(Not active on Bugzilla)]

Comment on attachment 8815685 [details]
Bug 1316008: Use explicit charset encoding

https://reviewboard.mozilla.org/r/96548/#review97050

Comment 7

[Adrian Zatreanu]

Cool :) I should wait for it to take a run on Try, right?

Comment 8

[Nevin Chen(Not active on Bugzilla)]

Sorry (In reply to Adrian Zatreanu from comment #7)
> Cool :) I should wait for it to take a run on Try, right?

I just run it for you :)

Comment 9

[Nevin Chen(Not active on Bugzilla)]

The test didn't pass. I've tried twice.
Maybe you can pull and rebase your code and try again?
https://treeherder.mozilla.org/#/jobs?repo=try&revision=b11ee952bda0d71464d80b394373005d8803e040
Thank you!

Comment 10

[Adrian Zatreanu]

Will try :)

Comment 11

[Adrian Zatreanu]

Comment on attachment 8815685 [details]
Bug 1316008: Use explicit charset encoding

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/96548/diff/1-2/

Comment 12

[Adrian Zatreanu]

Apparently, an import is done wrong. However, when im trying to sync with my Android Studio, im getting this: http://pastebin.com/kAT5rvMC

I'd like to fix this before fixing the issue in the patch, but no idea how. At some point, I could sync correctly. After the last pull, it doesn't work anymore.

Comment 13

[Nevin Chen(Not active on Bugzilla)]

Hi Alexander 
I've download the patch and built manually.
Could you please help me understand what may went wrong?

I saw in app module's build.gradle, there's a block for adding stumbler source code. May I know what it's about?

                if (mozconfig.substs.MOZ_ANDROID_MLS_STUMBLER) {
                    srcDir "${topsrcdir}/mobile/android/stumbler/java"
                }

The patch had some changes in stumbler source code and geckoview library. When I run ./mach build it failed with below message:

0:47.82 /Users/nechen/Desktop/mozilla-central/mobile/android/stumbler/java/org/mozilla/mozstumbler/service/stumblerthread/datahandling/DataStorageManager.java:10: error: package org.mozilla.gecko.util does not exist
 0:47.82 import org.mozilla.gecko.util.StringUtils;
 0:47.82                              ^
 0:47.86 /Users/nechen/Desktop/mozilla-central/mobile/android/stumbler/java/org/mozilla/mozstumbler/service/utils/Zipper.java:7: error: package org.mozilla.gecko.util does not exist
 0:47.86 import org.mozilla.gecko.util.StringUtils;
 0:47.86                              ^
 0:47.94 /Users/nechen/Desktop/mozilla-central/mobile/android/stumbler/java/org/mozilla/mozstumbler/service/stumblerthread/datahandling/DataStorageManager.java:106: error: cannot find symbol
 0:47.94                 bytes = Zipper.zipData(finalizeReports(mCurrentReports.reports).getBytes(StringUtils.UTF_8)).length;
 0:47.94                                                                                          ^
 0:47.94   symbol:   variable StringUtils
 0:47.94   location: class DataStorageManager
 0:47.95 /Users/nechen/Desktop/mozilla-central/mobile/android/stumbler/java/org/mozilla/mozstumbler/service/stumblerthread/datahandling/DataStorageManager.java:306: error: cannot find symbol
 0:47.95             final byte[] data = Zipper.zipData(finalizeReports(mCurrentReports.reports).getBytes(StringUtils.UTF_8));
 0:47.95                                                                                                  ^
 0:47.95   symbol:   variable StringUtils
 0:47.95   location: class DataStorageManager
 0:47.96 /Users/nechen/Desktop/mozilla-central/mobile/android/stumbler/java/org/mozilla/mozstumbler/service/stumblerthread/datahandling/DataStorageManager.java:417: error: cannot find symbol
 0:47.96         final byte[] bytes = Zipper.zipData(finalizeReports(mCurrentReports.reports).getBytes(StringUtils.UTF_8));
 0:47.96                                                                                               ^
 0:47.96   symbol:   variable StringUtils
 0:47.96   location: class DataStorageManager
 0:48.13 /Users/nechen/Desktop/mozilla-central/mobile/android/stumbler/java/org/mozilla/mozstumbler/service/utils/Zipper.java:38: error: cannot find symbol
 0:48.13             InputStreamReader reader = new InputStreamReader(gstream, StringUtils.UTF_8);
 0:48.13                                                                       ^
 0:48.13   symbol:   variable StringUtils
 0:48.13   location: class Zipper
 0:48.13 6 errors
 0:48.17 make[5]: *** [stumbler.jar] Error 1
 0:48.17 make[4]: *** [mobile/android/stumbler/libs] Error 2
 0:48.17 make[3]: *** [libs] Error 2
 0:48.17 make[2]: *** [default] Error 2
 0:48.18 make[1]: *** [realbuild] Error 2
 0:48.18 make: *** [build] Error 2
 0:48.20 0 compiler warnings present.

Comment 14

[Sebastian Kaspari (:sebastian; :pocmo)]

I guess you wanted to flag [e-mail]?

However the answer to that question is that the stumbler code is somewhat separate and therefore can't access any other packages. There's even a standalone version here:
https://github.com/mozilla/MozStumbler

To solve this the Stumbler code would need its own constant for now.

Comment 15

[Adrian Zatreanu]

Comment on attachment 8815685 [details]
Bug 1316008: Use explicit charset encoding

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/96548/diff/2-3/

Comment 16

[Adrian Zatreanu]

Comment on attachment 8815685 [details]
Bug 1316008: Use explicit charset encoding

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/96548/diff/3-4/

Comment 17

[Adrian Zatreanu]

Comment on attachment 8815685 [details]
Bug 1316008: Use explicit charset encoding

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/96548/diff/4-5/

Comment 18

[Adrian Zatreanu]

pushed for review :)

Comment 19

[Sebastian Kaspari (:sebastian; :pocmo)]

Pushed to try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=6a50627df392

Comment 20

[Adrian Zatreanu]

Comment on attachment 8815685 [details]
Bug 1316008: Use explicit charset encoding

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/96548/diff/5-6/

Comment 21

[Adrian Zatreanu]

Should be fixed now, ready to be tried :)

Comment 22

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8815685 [details]
Bug 1316008: Use explicit charset encoding

https://reviewboard.mozilla.org/r/96548/#review101246

I just saw that part of this commit modified a binary file, this should not be part of the commit:
> xpcom/tests/unit/data/SmallApp.app/Contents/MacOS/SmallApp

Comment 23

[Sebastian Kaspari (:sebastian; :pocmo)]

Pushed update to try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=188aefbbbd9d

Comment 24

[Adrian Zatreanu]

Comment on attachment 8815685 [details]
Bug 1316008: Use explicit charset encoding

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/96548/diff/6-7/

Comment 25

[Adrian Zatreanu]

Comment on attachment 8815685 [details]
Bug 1316008: Use explicit charset encoding

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/96548/diff/7-8/

Comment 26

[Adrian Zatreanu]

Comment on attachment 8815685 [details]
Bug 1316008: Use explicit charset encoding

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/96548/diff/8-9/

Comment 27

[Adrian Zatreanu]

Comment on attachment 8815685 [details]
Bug 1316008: Use explicit charset encoding

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/96548/diff/9-10/

Comment 28

[Adrian Zatreanu]

rebased (correctly i think) and removed the binary file from the commit, things should be fine now if the rebase was correct. if so, please trigger a try :) thanks

Comment 29

[Sebastian Kaspari (:sebastian; :pocmo)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=dece991ac1e3

Comment 30

[Adrian Zatreanu]

uhuu, looks like it passed :)

Comment 31

[Ryan VanderMeulen [:RyanVM]]

Please ensure that the patch has the proper reviews set in MozReview so it can autoland.

Comment 32

[Nevin Chen(Not active on Bugzilla)]

Comment on attachment 8815685 [details]
Bug 1316008: Use explicit charset encoding

https://reviewboard.mozilla.org/r/96548/#review101572

Comment 33

[Nevin Chen(Not active on Bugzilla)]

Hi Sebastian
Please help me.
Can I mark "checkin-needed" by myself? Or only you can do it?
Thanks!

Comment 34

[Carsten Book [:Tomcat]]

still need to fix the issue in mozreview so we can use the autoland feature

Comment 35

[Adrian Zatreanu]

im not really sure: is there anything needed to be done from me or :) ?

Comment 36

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8815685 [details]
Bug 1316008: Use explicit charset encoding

https://reviewboard.mozilla.org/r/96548/#review102432

Comment 37

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f05a9252d284
Use explicit charset encoding r=nechen,sebastian

Comment 38

[Sebastian Kaspari (:sebastian; :pocmo)]

I verified the patch locally and landed it. Thank you for your contribution! :)

Comment 39

[Adrian Zatreanu]

cool :)

Comment 40

[Phil Ringnalda (:philor)]

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