Closed
Bug 1316008
Opened 8 years ago
Closed 8 years ago
[findbugs] [Dm] Found reliance on default encoding
Categories
(Firefox for Android Graveyard :: General, defect, P3)
Tracking
(firefox53 fixed)
RESOLVED
FIXED
Firefox 53
Tracking | Status | |
---|---|---|
firefox53 | --- | fixed |
People
(Reporter: sebastian, Assigned: adrianzatreanu1, Mentored)
References
Details
(Whiteboard: [lang=java][good first bug])
Attachments
(1 file)
* 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•8 years 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•8 years 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•8 years ago
|
Assignee: nobody → adrianzatreanu1
Status: NEW → ASSIGNED
Assignee | ||
Updated•8 years ago
|
Attachment #8815685 -
Flags: review?(s.kaspari)
Reporter | ||
Updated•8 years ago
|
Attachment #8815685 -
Flags: review?(s.kaspari) → review?(cnevinchen)
Reporter | ||
Comment 4•8 years 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•8 years ago
|
||
Comment on attachment 8815685 [details] Bug 1316008: Use explicit charset encoding LGTM. Thanks!
Attachment #8815685 -
Flags: review+
Comment 6•8 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+
Comment 8•8 years 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•8 years 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 | ||
Updated•8 years ago
|
Flags: needinfo?(adrianzatreanu1)
Comment hidden (mozreview-request) |
Assignee | ||
Comment 12•8 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.
Comment 13•8 years 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•8 years 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) |
Reporter | ||
Comment 19•8 years ago
|
||
Pushed to try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=6a50627df392
Comment hidden (mozreview-request) |
Reporter | ||
Comment 22•8 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
Reporter | ||
Comment 23•8 years 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•8 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
Reporter | ||
Comment 29•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=dece991ac1e3
Updated•8 years ago
|
Keywords: checkin-needed
Comment 31•8 years ago
|
||
Please ensure that the patch has the proper reviews set in MozReview so it can autoland.
Keywords: checkin-needed
Comment 32•8 years ago
|
||
mozreview-review |
Comment on attachment 8815685 [details] Bug 1316008: Use explicit charset encoding https://reviewboard.mozilla.org/r/96548/#review101572
Comment 33•8 years 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
Comment 34•8 years ago
|
||
still need to fix the issue in mozreview so we can use the autoland feature
Keywords: checkin-needed
Assignee | ||
Comment 35•8 years ago
|
||
im not really sure: is there anything needed to be done from me or :) ?
Reporter | ||
Comment 36•8 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•8 years 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•8 years ago
|
||
I verified the patch locally and landed it. Thank you for your contribution! :)
Flags: needinfo?(s.kaspari)
Comment 40•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/f05a9252d284
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox53:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 53
Updated•4 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•