Closed Bug 1316008 Opened 8 years ago Closed 7 years ago

[findbugs] [Dm] Found reliance on default encoding

Categories

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

Product:
Firefox for Android Graveyard
Component:
General
Platform:
All
Android
Type:
defect

Tracking

(firefox53 fixed)

Status:
RESOLVED FIXED
Milestone:
Firefox 53
Tracking Flags:
Tracking Status
firefox53 --- fixed

People

(Reporter: sebastian, Assigned: adrianzatreanu1, Mentored)

References

Details

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

Attachments

(1 file)

Bug 1316008: Use explicit charset encoding
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
Assignee: nobody → adrianzatreanu1
Status: NEW → ASSIGNED
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 on attachment 8815685 [details]
Bug 1316008: Use explicit charset encoding

https://reviewboard.mozilla.org/r/96548/#review97050
Attachment #8815685 - Flags: review?(cnevinchen) → review+
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)
Will try :)
Flags: needinfo?(adrianzatreanu1)
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)
pushed for review :)
Should be fixed now, ready to be tried :)
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
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
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
im not really sure: is there anything needed to be done from me or :) ?
Comment on attachment 8815685 [details]
Bug 1316008: Use explicit charset encoding

https://reviewboard.mozilla.org/r/96548/#review102432
Attachment #8815685 - Flags: review+
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)
cool :)
https://hg.mozilla.org/mozilla-central/rev/f05a9252d284
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox53: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 53
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: