[findbugs] [Dm] Found reliance on default encoding

RESOLVED FIXED in Firefox 53

Status

()

Firefox for Android
General
P3
normal
RESOLVED FIXED
7 months ago
5 months ago

People

(Reporter: sebastian, Assigned: Adrian Zatreanu, Mentored)

Tracking

(Blocks: 1 bug)

unspecified
Firefox 53
All
Android
Points:
---

Firefox Tracking Flags

(firefox53 fixed)

Details

(Whiteboard: [lang=java][good first bug])

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

58 bytes, text/x-review-board-request
nechen
: review+
sebastian
: review+
Details | Review
(Reporter)

Description

7 months ago
* 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)
(Reporter)

Comment 1

7 months ago
"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."
(Reporter)

Comment 2

7 months ago
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 hidden (mozreview-request)
(Reporter)

Updated

6 months ago
Assignee: nobody → adrianzatreanu1
Status: NEW → ASSIGNED
(Assignee)

Updated

6 months ago
Attachment #8815685 - Flags: review?(s.kaspari)
(Reporter)

Updated

6 months ago
Attachment #8815685 - Flags: review?(s.kaspari) → review?(cnevinchen)
(Reporter)

Comment 4

6 months ago
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

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

LGTM. Thanks!
Attachment #8815685 - Flags: review+

Comment 6

6 months ago
mozreview-review
Comment on attachment 8815685 [details]
Bug 1316008: Use explicit charset encoding

https://reviewboard.mozilla.org/r/96548/#review97050
Attachment #8815685 - Flags: review?(cnevinchen) → review+
(Assignee)

Comment 7

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

Comment 8

6 months ago
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

6 months ago
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!
Flags: needinfo?(adrianzatreanu1)
(Assignee)

Comment 10

6 months ago
Will try :)
(Assignee)

Updated

6 months ago
Flags: needinfo?(adrianzatreanu1)
Comment hidden (mozreview-request)
(Assignee)

Comment 12

6 months ago
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

6 months ago
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.
Flags: needinfo?(nalexand)
(Reporter)

Comment 14

6 months ago
I guess you wanted to flag nalexander@mozilla.com?

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.
Flags: needinfo?(nalexand)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Comment 18

5 months ago
pushed for review :)
(Reporter)

Comment 19

5 months ago
Pushed to try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=6a50627df392
Comment hidden (mozreview-request)
(Assignee)

Comment 21

5 months ago
Should be fixed now, ready to be tried :)
(Reporter)

Comment 22

5 months ago
mozreview-review
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
(Reporter)

Comment 23

5 months ago
Pushed update to try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=188aefbbbd9d
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Comment 28

5 months ago
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
(Reporter)

Comment 29

5 months ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=dece991ac1e3
(Assignee)

Comment 30

5 months ago
uhuu, looks like it passed :)

Updated

5 months ago
Keywords: checkin-needed
Please ensure that the patch has the proper reviews set in MozReview so it can autoland.
Keywords: checkin-needed

Comment 32

5 months ago
mozreview-review
Comment on attachment 8815685 [details]
Bug 1316008: Use explicit charset encoding

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

Comment 33

5 months ago
Hi Sebastian
Please help me.
Can I mark "checkin-needed" by myself? Or only you can do it?
Thanks!
Flags: needinfo?(s.kaspari)
Keywords: checkin-needed
still need to fix the issue in mozreview so we can use the autoland feature
Keywords: checkin-needed
(Assignee)

Comment 35

5 months ago
im not really sure: is there anything needed to be done from me or :) ?
(Reporter)

Comment 36

5 months ago
mozreview-review
Comment on attachment 8815685 [details]
Bug 1316008: Use explicit charset encoding

https://reviewboard.mozilla.org/r/96548/#review102432
Attachment #8815685 - Flags: review+

Comment 37

5 months ago
Pushed by s.kaspari@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/f05a9252d284
Use explicit charset encoding r=nechen,sebastian
(Reporter)

Comment 38

5 months ago
I verified the patch locally and landed it. Thank you for your contribution! :)
Flags: needinfo?(s.kaspari)
(Assignee)

Comment 39

5 months ago
cool :)

Comment 40

5 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/f05a9252d284
Status: ASSIGNED → RESOLVED
Last Resolved: 5 months ago
status-firefox53: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 53
You need to log in before you can comment on or make changes to this bug.