[findbugs] [Dm] Found reliance on default encoding

RESOLVED FIXED in Firefox 53

Status

()

defect
P3
normal
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: sebastian, Assigned: adrianzatreanu1, Mentored)

Tracking

(Blocks 1 bug)

unspecified
Firefox 53
All
Android
Points:
---

Firefox Tracking Flags

(firefox53 fixed)

Details

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

Attachments

(1 attachment)

58 bytes, text/x-review-board-request
cnevinchen
: review+
sebastian
: review+
Details
* 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)
"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."
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)
Assignee: nobody → adrianzatreanu1
Status: NEW → ASSIGNED
(Assignee)

Updated

2 years ago
Attachment #8815685 - Flags: review?(s.kaspari)
Attachment #8815685 - Flags: review?(s.kaspari) → review?(cnevinchen)
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 on attachment 8815685 [details]
Bug 1316008: Use explicit charset encoding

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

Comment 6

2 years 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

2 years ago
Cool :) I should wait for it to take a run on Try, right?
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 :)
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

2 years ago
Will try :)
(Assignee)

Updated

2 years ago
Flags: needinfo?(adrianzatreanu1)
Comment hidden (mozreview-request)
(Assignee)

Comment 12

2 years 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.
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)
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

2 years ago
pushed for review :)
Comment hidden (mozreview-request)
(Assignee)

Comment 21

2 years ago
Should be fixed now, ready to be tried :)
(Reporter)

Comment 22

2 years 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
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Comment 28

2 years 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
(Assignee)

Comment 30

2 years ago
uhuu, looks like it passed :)
Please ensure that the patch has the proper reviews set in MozReview so it can autoland.
Keywords: checkin-needed
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

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

Comment 36

2 years 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

2 years ago
Pushed by s.kaspari@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/f05a9252d284
Use explicit charset encoding r=nechen,sebastian
I verified the patch locally and landed it. Thank you for your contribution! :)
Flags: needinfo?(s.kaspari)
(Assignee)

Comment 39

2 years ago
cool :)

Comment 40

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