Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(343)

Issue 246423004: Fix client certificate regressions on Android < 4.2 (Closed)

Created:
6 years, 8 months ago by tsniatowski
Modified:
6 years, 8 months ago
CC:
chromium-reviews, cbentzel+watch_chromium.org, Yang Gu
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Visibility:
Public.

Description

Fix client certificate regressions on Android < 4.2 https://codereview.chromium.org/166143002 refactored the code to use the AndroidPrivateKey wrapper class rather than Java's PrivateKey directly, but this one line of code was not updated. As a result, the test will always fail and client certificates won't work on Android versions prior to 4.2 (this is a compatibility code path, which is probably why it wasn't caught in testing or review). In addition, while https://codereview.chromium.org/182933002 updated the code to to fix 64-bit compilation issues, it missed the point that this is compatibility code meant to run only on earlier versions of Android that aren't 64-bit safe to begin with. As a result, a method called via reflection will return an unexpected integer instead of a long, causing the code to fail (even in 32-bit mode). Fix by conservatively casting the return value through Number instead, which will work with both int and long. (The earlier Android versions that this code is targeting are still not 64-bit safe, but the compatibility code should not cause any compile issues.) Investigation credit goes out to kimn@opera.com. BUG=360406 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=265531

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+7 lines, -3 lines) Patch
M net/android/java/src/org/chromium/net/DefaultAndroidKeyStore.java View 3 chunks +7 lines, -3 lines 0 comments Download

Messages

Total messages: 13 (0 generated)
tsniatowski
6 years, 8 months ago (2014-04-22 05:45:42 UTC) #1
Yaron
On 2014/04/22 05:45:42, tsniatowski wrote: lgtm Great catch! Not surprising that instanceof and reflection come ...
6 years, 8 months ago (2014-04-22 18:17:03 UTC) #2
Yaron
The CQ bit was checked by yfriedman@chromium.org
6 years, 8 months ago (2014-04-22 18:17:06 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/tsniatowski@opera.com/246423004/1
6 years, 8 months ago (2014-04-22 18:17:47 UTC) #4
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 8 months ago (2014-04-22 19:03:04 UTC) #5
commit-bot: I haz the power
Try jobs failed on following builders: tryserver.chromium on linux_chromium_chromeos_rel
6 years, 8 months ago (2014-04-22 19:03:05 UTC) #6
Yaron
The CQ bit was checked by yfriedman@chromium.org
6 years, 8 months ago (2014-04-22 19:12:22 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/tsniatowski@opera.com/246423004/1
6 years, 8 months ago (2014-04-22 19:14:53 UTC) #8
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 8 months ago (2014-04-23 00:01:59 UTC) #9
commit-bot: I haz the power
Try jobs failed on following builders: tryserver.chromium on win_chromium_rel
6 years, 8 months ago (2014-04-23 00:01:59 UTC) #10
Yaron
The CQ bit was checked by yfriedman@chromium.org
6 years, 8 months ago (2014-04-23 00:33:48 UTC) #11
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/tsniatowski@opera.com/246423004/1
6 years, 8 months ago (2014-04-23 00:34:50 UTC) #12
commit-bot: I haz the power
6 years, 8 months ago (2014-04-23 04:18:54 UTC) #13
Message was sent while issue was closed.
Change committed as 265531

Powered by Google App Engine
This is Rietveld 408576698