Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1485973002:
Removal of geolocation APIs on insecure origins (Closed)
|
Created:
5 years ago by jww Modified:
5 years ago CC:
blink-reviews, chromium-reviews, jochen (gone - plz use gerrit), mlamouri+watch-blink_chromium.org, mvanouwerkerk+watch_chromium.org, timvolodine Base URL:
https://chromium.googlesource.com/chromium/src@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionRemoval of geolocation APIs on insecure origins
This disallows the geolocation APIs getCurrentPosition() and
watchPosition() from being used on insecure origins. Adds a console
warning message that the API call has failed because of this.
BUG=520765, 561641
Committed: https://crrev.com/33ef9f5c8df422b0320cbc506d57bdce2999ebc8
Cr-Commit-Position: refs/heads/master@{#364642}
Patch Set 1 #
Total comments: 4
Patch Set 2 : Address philipj's nits #Patch Set 3 : Update GeolocationPermissionContext #Patch Set 4 : Fix unit tests #Patch Set 5 : Rebase on ToT #Patch Set 6 : Fix WebView tests #Messages
Total messages: 90 (28 generated)
jww@chromium.org changed reviewers: + philipj@opera.com
philipj@opera.com, can you take a look at this? This has the required 3 LGTMs from API owners on blink-dev, although I'm going to wait for one more final confirmation before committing. https://groups.google.com/a/chromium.org/d/msg/blink-dev/ylz0Zoph76A/jaMAcld6...
jww@chromium.org changed reviewers: + mlamouri@chromium.org
Whoops, meant to include mlamouri@ as a reviewer for the Geolocation bits :-) Sorry!
lgtm with optional testharness.js nits https://codereview.chromium.org/1485973002/diff/1/third_party/WebKit/LayoutTe... File third_party/WebKit/LayoutTests/http/tests/security/powerfulFeatureRestrictions/old-powerful-features-on-insecure-origin.html (right): https://codereview.chromium.org/1485973002/diff/1/third_party/WebKit/LayoutTe... third_party/WebKit/LayoutTests/http/tests/security/powerfulFeatureRestrictions/old-powerful-features-on-insecure-origin.html:60: this.done(); This line won't be reached, because assert_unreached throws an exception. This whole block can be replaced by this.unreached_func('getCurrentPosition should fail, but succeeded.') https://codereview.chromium.org/1485973002/diff/1/third_party/WebKit/LayoutTe... third_party/WebKit/LayoutTests/http/tests/security/powerfulFeatureRestrictions/old-powerful-features-on-insecure-origin.html:62: this.step_func(function(error) { And this could be just this.step_func_done().
Confirmed with rbyers that his lgtm is still gtg, so once mlamouri reviews, we should be good to go. https://codereview.chromium.org/1485973002/diff/1/third_party/WebKit/LayoutTe... File third_party/WebKit/LayoutTests/http/tests/security/powerfulFeatureRestrictions/old-powerful-features-on-insecure-origin.html (right): https://codereview.chromium.org/1485973002/diff/1/third_party/WebKit/LayoutTe... third_party/WebKit/LayoutTests/http/tests/security/powerfulFeatureRestrictions/old-powerful-features-on-insecure-origin.html:60: this.done(); On 2015/12/01 08:45:21, philipj wrote: > This line won't be reached, because assert_unreached throws an exception. This > whole block can be replaced by this.unreached_func('getCurrentPosition should > fail, but succeeded.') Done. https://codereview.chromium.org/1485973002/diff/1/third_party/WebKit/LayoutTe... third_party/WebKit/LayoutTests/http/tests/security/powerfulFeatureRestrictions/old-powerful-features-on-insecure-origin.html:62: this.step_func(function(error) { On 2015/12/01 08:45:21, philipj wrote: > And this could be just this.step_func_done(). Done.
Could you modify the permission context too?
Mounir, can you clarify? I'm not sure what you're looking for. On Tue, Dec 1, 2015 at 11:06 AM <mlamouri@chromium.org> wrote: > Could you modify the permission context too? > > https://codereview.chromium.org/1485973002/ > -- You received this message because you are subscribed to the Google Groups "Blink Reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to blink-reviews+unsubscribe@chromium.org.
Mounir, can you clarify? I'm not sure what you're looking for. On Tue, Dec 1, 2015 at 11:06 AM <mlamouri@chromium.org> wrote: > Could you modify the permission context too? > > https://codereview.chromium.org/1485973002/ > -- You received this message because you are subscribed to the Google Groups "Chromium-reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
Sorry for the lack of details, I assumed you knew about PermissionContexts. The content layer has a content::PermissionManager that should be implemented by the content/ embedder. For example, Chrome and Opera have different implementations of this. In chrome/, we use a PermissionContext instance for each permission. Geolocation has one: https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/geo... PermissionContextBase has a IsRestrictedToSecureOrigins() method that can be overridden by specific permission contexts to block permissions from insecure origins: https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/per... I think the GeolocationPermissionContext should implement the method and return true. Actually, I wonder if the decision should happen in Blink. It seems that we would force other content/ embedders and might make their permission model out of sync with what Blink does. This said, as Blink OWNERS and Opera person, I will leave this for Philip to decide.
On 2015/12/02 15:08:53, Mounir Lamouri OOO till Monday wrote: > Sorry for the lack of details, I assumed you knew about PermissionContexts. > > The content layer has a content::PermissionManager that should be implemented by > the content/ embedder. For example, Chrome and Opera have different > implementations of this. In chrome/, we use a PermissionContext instance for > each permission. Geolocation has one: > https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/geo... > > PermissionContextBase has a IsRestrictedToSecureOrigins() method that can be > overridden by specific permission contexts to block permissions from insecure > origins: > https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/per... > > I think the GeolocationPermissionContext should implement the method and return > true. > > Actually, I wonder if the decision should happen in Blink. It seems that we > would force other content/ embedders and might make their permission model out > of sync with what Blink does. This said, as Blink OWNERS and Opera person, I > will leave this for Philip to decide. Ah, got it! Thanks for pointing this out, Mounir. I'll upload a CL momentarily that makes it return |true| for Geolocation. However, I don't think the actually blocking should be implemented in the content layer. This is definitely a Blink-change (with Blink API approval), and it's matching a Web platform spec (Secure Contexts).
Mounir, can you take another look? Thanks!
lgtm
jww@chromium.org changed reviewers: + thestig@chromium.org
thestig@, can you OWNER review chrome/browser/geolocation/geolocation_permission_context.cc? Thanks!
lgtm
The CQ bit was checked by jww@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from philipj@opera.com Link to the patchset: https://codereview.chromium.org/1485973002/#ps40001 (title: "Update GeolocationPermissionContext")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1485973002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1485973002/40001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_chromeos_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
The CQ bit was checked by jww@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from thestig@chromium.org, philipj@opera.com, mlamouri@chromium.org Link to the patchset: https://codereview.chromium.org/1485973002/#ps60001 (title: "Fix unit tests")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1485973002/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1485973002/60001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: win8_chromium_ng on tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_ng/...)
The CQ bit was checked by jww@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from thestig@chromium.org, philipj@opera.com, mlamouri@chromium.org Link to the patchset: https://codereview.chromium.org/1485973002/#ps80001 (title: "Rebase on ToT")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1485973002/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1485973002/80001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
The CQ bit was checked by jww@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1485973002/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1485973002/80001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_asan_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by jww@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1485973002/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1485973002/80001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_android_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_android_r...)
The CQ bit was checked by jww@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1485973002/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1485973002/80001
The CQ bit was checked by jww@chromium.org to run a CQ dry run
jww@chromium.org changed reviewers: + sgurun@chromium.org
sgurun@chromium.org, would you mind taking a look at my WebView test change? Thanks!
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1485973002/100001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1485973002/100001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
On 2015/12/09 05:27:10, commit-bot: I haz the power wrote: > Dry run: This issue passed the CQ dry run. And, sgurun@, just to clarify why this change is necessary... This CL makes Geolocation only available from secure origins. Top-level data: URIs are not considered secure, and these WebView tests were testing Geolocation behavior by injecting data: URIs at the top level. My change to the WebView test is to make the injecting occur on an HTTPS URI instead of a top-level data: URI, thus making them appear to come from a secure origin so the Geolocation APIs are available.
On 2015/12/09 16:22:38, jww wrote: > On 2015/12/09 05:27:10, commit-bot: I haz the power wrote: > > Dry run: This issue passed the CQ dry run. > > And, sgurun@, just to clarify why this change is necessary... This CL makes > Geolocation only available from secure origins. Top-level data: URIs are not > considered secure, and these WebView tests were testing Geolocation behavior by > injecting data: URIs at the top level. My change to the WebView test is to make > the injecting occur on an HTTPS URI instead of a top-level data: URI, thus > making them appear to come from a secure origin so the Geolocation APIs are > available. will look at it today
On 2015/12/09 16:24:08, sgurun wrote: > On 2015/12/09 16:22:38, jww wrote: > > On 2015/12/09 05:27:10, commit-bot: I haz the power wrote: > > > Dry run: This issue passed the CQ dry run. > > > > And, sgurun@, just to clarify why this change is necessary... This CL makes > > Geolocation only available from secure origins. Top-level data: URIs are not > > considered secure, and these WebView tests were testing Geolocation behavior > by > > injecting data: URIs at the top level. My change to the WebView test is to > make > > the injecting occur on an HTTPS URI instead of a top-level data: URI, thus > > making them appear to come from a secure origin so the Geolocation APIs are > > available. > > will look at it today Hello, this has the potential to break android webview apps using geolocation in this manner I think. so any app using geolocation via data: uri's will be broken right?
On 2015/12/09 19:17:33, sgurun wrote: > On 2015/12/09 16:24:08, sgurun wrote: > > On 2015/12/09 16:22:38, jww wrote: > > > On 2015/12/09 05:27:10, commit-bot: I haz the power wrote: > > > > Dry run: This issue passed the CQ dry run. > > > > > > And, sgurun@, just to clarify why this change is necessary... This CL makes > > > Geolocation only available from secure origins. Top-level data: URIs are not > > > considered secure, and these WebView tests were testing Geolocation behavior > > by > > > injecting data: URIs at the top level. My change to the WebView test is to > > make > > > the injecting occur on an HTTPS URI instead of a top-level data: URI, thus > > > making them appear to come from a secure origin so the Geolocation APIs are > > > available. > > > > will look at it today > > Hello, > > this has the potential to break android webview apps using geolocation in this > manner I think. > so any app using geolocation via data: uri's will be broken right? Correct. From Blink's perspective, since there's no way to determine where it originated, a data: URI is insecure (for example, imagine an http: site window.open()'d a data: URI). As mentioned at the top, our UMA stats show that, in total, use of geolocation in insecure origins is very low, so we expect very low impact.
On 2015/12/09 20:54:27, jww wrote: > On 2015/12/09 19:17:33, sgurun wrote: > > On 2015/12/09 16:24:08, sgurun wrote: > > > On 2015/12/09 16:22:38, jww wrote: > > > > On 2015/12/09 05:27:10, commit-bot: I haz the power wrote: > > > > > Dry run: This issue passed the CQ dry run. > > > > > > > > And, sgurun@, just to clarify why this change is necessary... This CL > makes > > > > Geolocation only available from secure origins. Top-level data: URIs are > not > > > > considered secure, and these WebView tests were testing Geolocation > behavior > > > by > > > > injecting data: URIs at the top level. My change to the WebView test is to > > > make > > > > the injecting occur on an HTTPS URI instead of a top-level data: URI, thus > > > > making them appear to come from a secure origin so the Geolocation APIs > are > > > > available. > > > > > > will look at it today > > > > Hello, > > > > this has the potential to break android webview apps using geolocation in this > > manner I think. > > so any app using geolocation via data: uri's will be broken right? > > Correct. From Blink's perspective, since there's no way to determine where it > originated, a data: URI is insecure (for example, imagine an http: site > window.open()'d a data: URI). As mentioned at the top, our UMA stats show that, > in total, use of geolocation in insecure origins is very low, so we expect very > low impact. but webview does not have UMA, so we cannot know its impact by looking at UMA stats. The problem is not how widespread the usage is, if a few very widely used apps are using this techique, they will start failing after the webview update. The users will have no idea what broke them We have two different possible paths here: 1. do nothing and enforce the change, and hope anything important will be caught in the beta timeline 2. provide a way for webview not to enforce the change until next Android release and enforce the change only to the apps targeting that. I am tending towards 2
On 2015/12/09 21:01:57, sgurun wrote: > On 2015/12/09 20:54:27, jww wrote: > > On 2015/12/09 19:17:33, sgurun wrote: > > > On 2015/12/09 16:24:08, sgurun wrote: > > > > On 2015/12/09 16:22:38, jww wrote: > > > > > On 2015/12/09 05:27:10, commit-bot: I haz the power wrote: > > > > > > Dry run: This issue passed the CQ dry run. > > > > > > > > > > And, sgurun@, just to clarify why this change is necessary... This CL > > makes > > > > > Geolocation only available from secure origins. Top-level data: URIs are > > not > > > > > considered secure, and these WebView tests were testing Geolocation > > behavior > > > > by > > > > > injecting data: URIs at the top level. My change to the WebView test is > to > > > > make > > > > > the injecting occur on an HTTPS URI instead of a top-level data: URI, > thus > > > > > making them appear to come from a secure origin so the Geolocation APIs > > are > > > > > available. > > > > > > > > will look at it today > > > > > > Hello, > > > > > > this has the potential to break android webview apps using geolocation in > this > > > manner I think. > > > so any app using geolocation via data: uri's will be broken right? > > > > Correct. From Blink's perspective, since there's no way to determine where it > > originated, a data: URI is insecure (for example, imagine an http: site > > window.open()'d a data: URI). As mentioned at the top, our UMA stats show > that, > > in total, use of geolocation in insecure origins is very low, so we expect > very > > low impact. > > but webview does not have UMA, so we cannot know its impact by looking at UMA > stats. The problem is not how widespread the usage is, if a few very widely used > apps are using this techique, they will start failing after the webview update. > The users will have no idea what broke them > > We have two different possible paths here: > 1. do nothing and enforce the change, and hope anything important will be caught > in the beta timeline > 2. provide a way for webview not to enforce the change until next Android > release and enforce the change only to the apps targeting that. > I am tending towards 2 If we think there's non-trivial risk here then I agree #2 is probably the right option. aelias@ tells me that for an example of a targetSdkVersion-based quirk plumbing, grep for "use_legacy_background_size_shorthand_behavior" (useLegacyBackgroundSizeShorthandBehavior on the Blink side).
On 2015/12/09 21:12:45, Rick Byers wrote: > On 2015/12/09 21:01:57, sgurun wrote: > > On 2015/12/09 20:54:27, jww wrote: > > > On 2015/12/09 19:17:33, sgurun wrote: > > > > On 2015/12/09 16:24:08, sgurun wrote: > > > > > On 2015/12/09 16:22:38, jww wrote: > > > > > > On 2015/12/09 05:27:10, commit-bot: I haz the power wrote: > > > > > > > Dry run: This issue passed the CQ dry run. > > > > > > > > > > > > And, sgurun@, just to clarify why this change is necessary... This CL > > > makes > > > > > > Geolocation only available from secure origins. Top-level data: URIs > are > > > not > > > > > > considered secure, and these WebView tests were testing Geolocation > > > behavior > > > > > by > > > > > > injecting data: URIs at the top level. My change to the WebView test > is > > to > > > > > make > > > > > > the injecting occur on an HTTPS URI instead of a top-level data: URI, > > thus > > > > > > making them appear to come from a secure origin so the Geolocation > APIs > > > are > > > > > > available. > > > > > > > > > > will look at it today > > > > > > > > Hello, > > > > > > > > this has the potential to break android webview apps using geolocation in > > this > > > > manner I think. > > > > so any app using geolocation via data: uri's will be broken right? > > > > > > Correct. From Blink's perspective, since there's no way to determine where > it > > > originated, a data: URI is insecure (for example, imagine an http: site > > > window.open()'d a data: URI). As mentioned at the top, our UMA stats show > > that, > > > in total, use of geolocation in insecure origins is very low, so we expect > > very > > > low impact. > > > > but webview does not have UMA, so we cannot know its impact by looking at UMA > > stats. The problem is not how widespread the usage is, if a few very widely > used > > apps are using this techique, they will start failing after the webview > update. > > The users will have no idea what broke them > > > > We have two different possible paths here: > > 1. do nothing and enforce the change, and hope anything important will be > caught > > in the beta timeline > > 2. provide a way for webview not to enforce the change until next Android > > release and enforce the change only to the apps targeting that. > > I am tending towards 2 > > If we think there's non-trivial risk here then I agree #2 is probably the right > option. > > aelias@ tells me that for an example of a targetSdkVersion-based quirk plumbing, > grep for "use_legacy_background_size_shorthand_behavior" > (useLegacyBackgroundSizeShorthandBehavior on the Blink side). I think going safe is the better approach here since I do not have any data to back option 1 up.
On 2015/12/09 22:30:46, sgurun wrote: > On 2015/12/09 21:12:45, Rick Byers wrote: > > On 2015/12/09 21:01:57, sgurun wrote: > > > On 2015/12/09 20:54:27, jww wrote: > > > > On 2015/12/09 19:17:33, sgurun wrote: > > > > > On 2015/12/09 16:24:08, sgurun wrote: > > > > > > On 2015/12/09 16:22:38, jww wrote: > > > > > > > On 2015/12/09 05:27:10, commit-bot: I haz the power wrote: > > > > > > > > Dry run: This issue passed the CQ dry run. > > > > > > > > > > > > > > And, sgurun@, just to clarify why this change is necessary... This > CL > > > > makes > > > > > > > Geolocation only available from secure origins. Top-level data: URIs > > are > > > > not > > > > > > > considered secure, and these WebView tests were testing Geolocation > > > > behavior > > > > > > by > > > > > > > injecting data: URIs at the top level. My change to the WebView test > > is > > > to > > > > > > make > > > > > > > the injecting occur on an HTTPS URI instead of a top-level data: > URI, > > > thus > > > > > > > making them appear to come from a secure origin so the Geolocation > > APIs > > > > are > > > > > > > available. > > > > > > > > > > > > will look at it today > > > > > > > > > > Hello, > > > > > > > > > > this has the potential to break android webview apps using geolocation > in > > > this > > > > > manner I think. > > > > > so any app using geolocation via data: uri's will be broken right? > > > > > > > > Correct. From Blink's perspective, since there's no way to determine where > > it > > > > originated, a data: URI is insecure (for example, imagine an http: site > > > > window.open()'d a data: URI). As mentioned at the top, our UMA stats show > > > that, > > > > in total, use of geolocation in insecure origins is very low, so we expect > > > very > > > > low impact. > > > > > > but webview does not have UMA, so we cannot know its impact by looking at > UMA > > > stats. The problem is not how widespread the usage is, if a few very widely > > used > > > apps are using this techique, they will start failing after the webview > > update. > > > The users will have no idea what broke them > > > > > > We have two different possible paths here: > > > 1. do nothing and enforce the change, and hope anything important will be > > caught > > > in the beta timeline > > > 2. provide a way for webview not to enforce the change until next Android > > > release and enforce the change only to the apps targeting that. > > > I am tending towards 2 > > > > If we think there's non-trivial risk here then I agree #2 is probably the > right > > option. > > > > aelias@ tells me that for an example of a targetSdkVersion-based quirk > plumbing, > > grep for "use_legacy_background_size_shorthand_behavior" > > (useLegacyBackgroundSizeShorthandBehavior on the Blink side). > > I think going safe is the better approach here since I do not have any data to > back option 1 up. I'm going to talk to some other security folks about this because option #2 doesn't sound great to me. Since we don't have a way to really separate out data: URIs, what we're really talking about is indefinitely allowing Geolocation on insecure origins for some portion of WebView, which is a pretty hefty exception. Given that we're pulling the band-aid off for the Web, I don't see why we wouldn't pull the band-aid off for WebView as well.
On 2015/12/09 23:32:14, jww wrote: > On 2015/12/09 22:30:46, sgurun wrote: > > On 2015/12/09 21:12:45, Rick Byers wrote: > > > On 2015/12/09 21:01:57, sgurun wrote: > > > > On 2015/12/09 20:54:27, jww wrote: > > > > > On 2015/12/09 19:17:33, sgurun wrote: > > > > > > On 2015/12/09 16:24:08, sgurun wrote: > > > > > > > On 2015/12/09 16:22:38, jww wrote: > > > > > > > > On 2015/12/09 05:27:10, commit-bot: I haz the power wrote: > > > > > > > > > Dry run: This issue passed the CQ dry run. > > > > > > > > > > > > > > > > And, sgurun@, just to clarify why this change is necessary... This > > CL > > > > > makes > > > > > > > > Geolocation only available from secure origins. Top-level data: > URIs > > > are > > > > > not > > > > > > > > considered secure, and these WebView tests were testing > Geolocation > > > > > behavior > > > > > > > by > > > > > > > > injecting data: URIs at the top level. My change to the WebView > test > > > is > > > > to > > > > > > > make > > > > > > > > the injecting occur on an HTTPS URI instead of a top-level data: > > URI, > > > > thus > > > > > > > > making them appear to come from a secure origin so the Geolocation > > > APIs > > > > > are > > > > > > > > available. > > > > > > > > > > > > > > will look at it today > > > > > > > > > > > > Hello, > > > > > > > > > > > > this has the potential to break android webview apps using geolocation > > in > > > > this > > > > > > manner I think. > > > > > > so any app using geolocation via data: uri's will be broken right? > > > > > > > > > > Correct. From Blink's perspective, since there's no way to determine > where > > > it > > > > > originated, a data: URI is insecure (for example, imagine an http: site > > > > > window.open()'d a data: URI). As mentioned at the top, our UMA stats > show > > > > that, > > > > > in total, use of geolocation in insecure origins is very low, so we > expect > > > > very > > > > > low impact. > > > > > > > > but webview does not have UMA, so we cannot know its impact by looking at > > UMA > > > > stats. The problem is not how widespread the usage is, if a few very > widely > > > used > > > > apps are using this techique, they will start failing after the webview > > > update. > > > > The users will have no idea what broke them > > > > > > > > We have two different possible paths here: > > > > 1. do nothing and enforce the change, and hope anything important will be > > > caught > > > > in the beta timeline > > > > 2. provide a way for webview not to enforce the change until next Android > > > > release and enforce the change only to the apps targeting that. > > > > I am tending towards 2 > > > > > > If we think there's non-trivial risk here then I agree #2 is probably the > > right > > > option. > > > > > > aelias@ tells me that for an example of a targetSdkVersion-based quirk > > plumbing, > > > grep for "use_legacy_background_size_shorthand_behavior" > > > (useLegacyBackgroundSizeShorthandBehavior on the Blink side). > > > > I think going safe is the better approach here since I do not have any data to > > back option 1 up. > > I'm going to talk to some other security folks about this because option #2 > doesn't sound great to me. Since we don't have a way to really separate out > data: URIs, what we're really talking about is indefinitely allowing Geolocation > on insecure origins for some portion of WebView, which is a pretty hefty > exception. Given that we're pulling the band-aid off for the Web, I don't see > why we wouldn't pull the band-aid off for WebView as well. sgurun and rbyers: Thinking about this a little further, how is this different from any other deprecation and removal we do in Blink? That is, we remove features all of the time, and they could be used in some special way in WebView, but we don't have UMA there, so we don't know. That is, we only see this as a problem because there happened to be a test that leverages this behavior in a special way. Take, for example, getUserMedia(), which we removed (from insecure origins) in m47. That had the exact same behavior as here, but we didn't special case it in WebView. Or take any of the other recent Blink removals, such as: * getComputedStyle(e).css: https://groups.google.com/a/chromium.org/forum/#!searchin/blink-dev/intent$20... * <applet>: https://groups.google.com/a/chromium.org/forum/#!searchin/blink-dev/intent$20... We didn't special case them for WebView. Is there a reason we should special case geolocation with WebView here? That is to say, I'm of the opinion that it would better to proceed with the deprecation and we can react as necessary. If it turns out to be a huge deal for WebView, we can react accordingly, just as we would plan to do for any other platform. Please let me know where I've gone off the deep end here :-)
On 2015/12/10 02:25:28, jww wrote: > On 2015/12/09 23:32:14, jww wrote: > > On 2015/12/09 22:30:46, sgurun wrote: > > > On 2015/12/09 21:12:45, Rick Byers wrote: > > > > On 2015/12/09 21:01:57, sgurun wrote: > > > > > On 2015/12/09 20:54:27, jww wrote: > > > > > > On 2015/12/09 19:17:33, sgurun wrote: > > > > > > > On 2015/12/09 16:24:08, sgurun wrote: > > > > > > > > On 2015/12/09 16:22:38, jww wrote: > > > > > > > > > On 2015/12/09 05:27:10, commit-bot: I haz the power wrote: > > > > > > > > > > Dry run: This issue passed the CQ dry run. > > > > > > > > > > > > > > > > > > And, sgurun@, just to clarify why this change is necessary... > This > > > CL > > > > > > makes > > > > > > > > > Geolocation only available from secure origins. Top-level data: > > URIs > > > > are > > > > > > not > > > > > > > > > considered secure, and these WebView tests were testing > > Geolocation > > > > > > behavior > > > > > > > > by > > > > > > > > > injecting data: URIs at the top level. My change to the WebView > > test > > > > is > > > > > to > > > > > > > > make > > > > > > > > > the injecting occur on an HTTPS URI instead of a top-level data: > > > URI, > > > > > thus > > > > > > > > > making them appear to come from a secure origin so the > Geolocation > > > > APIs > > > > > > are > > > > > > > > > available. > > > > > > > > > > > > > > > > will look at it today > > > > > > > > > > > > > > Hello, > > > > > > > > > > > > > > this has the potential to break android webview apps using > geolocation > > > in > > > > > this > > > > > > > manner I think. > > > > > > > so any app using geolocation via data: uri's will be broken right? > > > > > > > > > > > > Correct. From Blink's perspective, since there's no way to determine > > where > > > > it > > > > > > originated, a data: URI is insecure (for example, imagine an http: > site > > > > > > window.open()'d a data: URI). As mentioned at the top, our UMA stats > > show > > > > > that, > > > > > > in total, use of geolocation in insecure origins is very low, so we > > expect > > > > > very > > > > > > low impact. > > > > > > > > > > but webview does not have UMA, so we cannot know its impact by looking > at > > > UMA > > > > > stats. The problem is not how widespread the usage is, if a few very > > widely > > > > used > > > > > apps are using this techique, they will start failing after the webview > > > > update. > > > > > The users will have no idea what broke them > > > > > > > > > > We have two different possible paths here: > > > > > 1. do nothing and enforce the change, and hope anything important will > be > > > > caught > > > > > in the beta timeline > > > > > 2. provide a way for webview not to enforce the change until next > Android > > > > > release and enforce the change only to the apps targeting that. > > > > > I am tending towards 2 > > > > > > > > If we think there's non-trivial risk here then I agree #2 is probably the > > > right > > > > option. > > > > > > > > aelias@ tells me that for an example of a targetSdkVersion-based quirk > > > plumbing, > > > > grep for "use_legacy_background_size_shorthand_behavior" > > > > (useLegacyBackgroundSizeShorthandBehavior on the Blink side). > > > > > > I think going safe is the better approach here since I do not have any data > to > > > back option 1 up. > > > > I'm going to talk to some other security folks about this because option #2 > > doesn't sound great to me. Since we don't have a way to really separate out > > data: URIs, what we're really talking about is indefinitely allowing > Geolocation > > on insecure origins for some portion of WebView, which is a pretty hefty > > exception. Given that we're pulling the band-aid off for the Web, I don't see > > why we wouldn't pull the band-aid off for WebView as well. > > sgurun and rbyers: Thinking about this a little further, how is this different > from any other deprecation and removal we do in Blink? That is, we remove > features all of the time, and they could be used in some special way in WebView, > but we don't have UMA there, so we don't know. That is, we only see this as a > problem because there happened to be a test that leverages this behavior in a > special way. > > Take, for example, getUserMedia(), which we removed (from insecure origins) in > m47. That had the exact same behavior as here, but we didn't special case it in > WebView. > > Or take any of the other recent Blink removals, such as: > * getComputedStyle(e).css: > https://groups.google.com/a/chromium.org/forum/#!searchin/blink-dev/intent$20... > * <applet>: > https://groups.google.com/a/chromium.org/forum/#!searchin/blink-dev/intent$20... > We didn't special case them for WebView. Is there a reason we should special > case geolocation with WebView here? > > That is to say, I'm of the opinion that it would better to proceed with the > deprecation and we can react as necessary. If it turns out to be a huge deal for > WebView, we can react accordingly, just as we would plan to do for any other > platform. > > Please let me know where I've gone off the deep end here :-) It is not easy to draw a line. We need to improve security of the platform but we also need to keep the platform functioning. I guess you mean reverting or providing a quirk mode if it turns out to be a great deal. I am going to check our compatibility test suite (which is a test suite that guarantees api behavior) tomorrow to see what behavior we are testing there.
On 2015/12/10 03:28:27, sgurun wrote: > On 2015/12/10 02:25:28, jww wrote: > > On 2015/12/09 23:32:14, jww wrote: > > > On 2015/12/09 22:30:46, sgurun wrote: > > > > On 2015/12/09 21:12:45, Rick Byers wrote: > > > > > On 2015/12/09 21:01:57, sgurun wrote: > > > > > > On 2015/12/09 20:54:27, jww wrote: > > > > > > > On 2015/12/09 19:17:33, sgurun wrote: > > > > > > > > On 2015/12/09 16:24:08, sgurun wrote: > > > > > > > > > On 2015/12/09 16:22:38, jww wrote: > > > > > > > > > > On 2015/12/09 05:27:10, commit-bot: I haz the power wrote: > > > > > > > > > > > Dry run: This issue passed the CQ dry run. > > > > > > > > > > > > > > > > > > > > And, sgurun@, just to clarify why this change is necessary... > > This > > > > CL > > > > > > > makes > > > > > > > > > > Geolocation only available from secure origins. Top-level > data: > > > URIs > > > > > are > > > > > > > not > > > > > > > > > > considered secure, and these WebView tests were testing > > > Geolocation > > > > > > > behavior > > > > > > > > > by > > > > > > > > > > injecting data: URIs at the top level. My change to the > WebView > > > test > > > > > is > > > > > > to > > > > > > > > > make > > > > > > > > > > the injecting occur on an HTTPS URI instead of a top-level > data: > > > > URI, > > > > > > thus > > > > > > > > > > making them appear to come from a secure origin so the > > Geolocation > > > > > APIs > > > > > > > are > > > > > > > > > > available. > > > > > > > > > > > > > > > > > > will look at it today > > > > > > > > > > > > > > > > Hello, > > > > > > > > > > > > > > > > this has the potential to break android webview apps using > > geolocation > > > > in > > > > > > this > > > > > > > > manner I think. > > > > > > > > so any app using geolocation via data: uri's will be broken right? > > > > > > > > > > > > > > Correct. From Blink's perspective, since there's no way to determine > > > where > > > > > it > > > > > > > originated, a data: URI is insecure (for example, imagine an http: > > site > > > > > > > window.open()'d a data: URI). As mentioned at the top, our UMA stats > > > show > > > > > > that, > > > > > > > in total, use of geolocation in insecure origins is very low, so we > > > expect > > > > > > very > > > > > > > low impact. > > > > > > > > > > > > but webview does not have UMA, so we cannot know its impact by looking > > at > > > > UMA > > > > > > stats. The problem is not how widespread the usage is, if a few very > > > widely > > > > > used > > > > > > apps are using this techique, they will start failing after the > webview > > > > > update. > > > > > > The users will have no idea what broke them > > > > > > > > > > > > We have two different possible paths here: > > > > > > 1. do nothing and enforce the change, and hope anything important will > > be > > > > > caught > > > > > > in the beta timeline > > > > > > 2. provide a way for webview not to enforce the change until next > > Android > > > > > > release and enforce the change only to the apps targeting that. > > > > > > I am tending towards 2 > > > > > > > > > > If we think there's non-trivial risk here then I agree #2 is probably > the > > > > right > > > > > option. > > > > > > > > > > aelias@ tells me that for an example of a targetSdkVersion-based quirk > > > > plumbing, > > > > > grep for "use_legacy_background_size_shorthand_behavior" > > > > > (useLegacyBackgroundSizeShorthandBehavior on the Blink side). > > > > > > > > I think going safe is the better approach here since I do not have any > data > > to > > > > back option 1 up. > > > > > > I'm going to talk to some other security folks about this because option #2 > > > doesn't sound great to me. Since we don't have a way to really separate out > > > data: URIs, what we're really talking about is indefinitely allowing > > Geolocation > > > on insecure origins for some portion of WebView, which is a pretty hefty > > > exception. Given that we're pulling the band-aid off for the Web, I don't > see > > > why we wouldn't pull the band-aid off for WebView as well. > > > > sgurun and rbyers: Thinking about this a little further, how is this different > > from any other deprecation and removal we do in Blink? That is, we remove > > features all of the time, and they could be used in some special way in > WebView, > > but we don't have UMA there, so we don't know. That is, we only see this as a > > problem because there happened to be a test that leverages this behavior in a > > special way. > > > > Take, for example, getUserMedia(), which we removed (from insecure origins) in > > m47. That had the exact same behavior as here, but we didn't special case it > in > > WebView. > > > > Or take any of the other recent Blink removals, such as: > > * getComputedStyle(e).css: > > > https://groups.google.com/a/chromium.org/forum/#!searchin/blink-dev/intent$20... > > * <applet>: > > > https://groups.google.com/a/chromium.org/forum/#!searchin/blink-dev/intent$20... > > We didn't special case them for WebView. Is there a reason we should special > > case geolocation with WebView here? > > > > That is to say, I'm of the opinion that it would better to proceed with the > > deprecation and we can react as necessary. If it turns out to be a huge deal > for > > WebView, we can react accordingly, just as we would plan to do for any other > > platform. > > > > Please let me know where I've gone off the deep end here :-) > > It is not easy to draw a line. We need to improve security of the platform but > we also need to keep the platform functioning. I guess you mean reverting or I completely agree, and it wasn't obvious that we would decide to deprecate and remove Geolocation. This is why we went through the Blink deprecation and removal process and the consensus was that the trade-off is worth the functional cost, which lead to API owner approval. On the other side, fullscreen on insecure origins was originally slated for deprecation and removal, but we undid that decision because the numbers were not good and it was not a huge security win. Do you feel that there's a different set of use cases for WebView then the general Web that warrant leaving a security exception in place or that make the Blink platform decision incorrect? If so, it would be great to outline those cases so we can understand them better and hopefully address any problems there so we can continue to move the Web to a more secure place. > providing a quirk mode if it turns out to be a great deal. > > I am going to check our compatibility test suite (which is a test suite that > guarantees api behavior) tomorrow to see what behavior we are testing there. Cool, thanks!
Description was changed from ========== Removal of geolocation APIs on insecure origins This disallows the geolocation APIs getCurrentPosition() and watchPosition() from being used on insecure origins. Adds a console warning message that the API call has failed because of this. BUG=520765,561641 ========== to ========== Removal of geolocation APIs on insecure origins This disallows the geolocation APIs getCurrentPosition() and watchPosition() from being used on insecure origins. Adds a console warning message that the API call has failed because of this. BUG=520765,561641 ==========
sgurun@chromium.org changed reviewers: + torne@chromium.org
On 2015/12/10 06:35:35, jww wrote: > On 2015/12/10 03:28:27, sgurun wrote: > > On 2015/12/10 02:25:28, jww wrote: > > > On 2015/12/09 23:32:14, jww wrote: > > > > On 2015/12/09 22:30:46, sgurun wrote: > > > > > On 2015/12/09 21:12:45, Rick Byers wrote: > > > > > > On 2015/12/09 21:01:57, sgurun wrote: > > > > > > > On 2015/12/09 20:54:27, jww wrote: > > > > > > > > On 2015/12/09 19:17:33, sgurun wrote: > > > > > > > > > On 2015/12/09 16:24:08, sgurun wrote: > > > > > > > > > > On 2015/12/09 16:22:38, jww wrote: > > > > > > > > > > > On 2015/12/09 05:27:10, commit-bot: I haz the power wrote: > > > > > > > > > > > > Dry run: This issue passed the CQ dry run. > > > > > > > > > > > > > > > > > > > > > > And, sgurun@, just to clarify why this change is > necessary... > > > This > > > > > CL > > > > > > > > makes > > > > > > > > > > > Geolocation only available from secure origins. Top-level > > data: > > > > URIs > > > > > > are > > > > > > > > not > > > > > > > > > > > considered secure, and these WebView tests were testing > > > > Geolocation > > > > > > > > behavior > > > > > > > > > > by > > > > > > > > > > > injecting data: URIs at the top level. My change to the > > WebView > > > > test > > > > > > is > > > > > > > to > > > > > > > > > > make > > > > > > > > > > > the injecting occur on an HTTPS URI instead of a top-level > > data: > > > > > URI, > > > > > > > thus > > > > > > > > > > > making them appear to come from a secure origin so the > > > Geolocation > > > > > > APIs > > > > > > > > are > > > > > > > > > > > available. > > > > > > > > > > > > > > > > > > > > will look at it today > > > > > > > > > > > > > > > > > > Hello, > > > > > > > > > > > > > > > > > > this has the potential to break android webview apps using > > > geolocation > > > > > in > > > > > > > this > > > > > > > > > manner I think. > > > > > > > > > so any app using geolocation via data: uri's will be broken > right? > > > > > > > > > > > > > > > > Correct. From Blink's perspective, since there's no way to > determine > > > > where > > > > > > it > > > > > > > > originated, a data: URI is insecure (for example, imagine an http: > > > site > > > > > > > > window.open()'d a data: URI). As mentioned at the top, our UMA > stats > > > > show > > > > > > > that, > > > > > > > > in total, use of geolocation in insecure origins is very low, so > we > > > > expect > > > > > > > very > > > > > > > > low impact. > > > > > > > > > > > > > > but webview does not have UMA, so we cannot know its impact by > looking > > > at > > > > > UMA > > > > > > > stats. The problem is not how widespread the usage is, if a few very > > > > widely > > > > > > used > > > > > > > apps are using this techique, they will start failing after the > > webview > > > > > > update. > > > > > > > The users will have no idea what broke them > > > > > > > > > > > > > > We have two different possible paths here: > > > > > > > 1. do nothing and enforce the change, and hope anything important > will > > > be > > > > > > caught > > > > > > > in the beta timeline > > > > > > > 2. provide a way for webview not to enforce the change until next > > > Android > > > > > > > release and enforce the change only to the apps targeting that. > > > > > > > I am tending towards 2 > > > > > > > > > > > > If we think there's non-trivial risk here then I agree #2 is probably > > the > > > > > right > > > > > > option. > > > > > > > > > > > > aelias@ tells me that for an example of a targetSdkVersion-based quirk > > > > > plumbing, > > > > > > grep for "use_legacy_background_size_shorthand_behavior" > > > > > > (useLegacyBackgroundSizeShorthandBehavior on the Blink side). > > > > > > > > > > I think going safe is the better approach here since I do not have any > > data > > > to > > > > > back option 1 up. > > > > > > > > I'm going to talk to some other security folks about this because option > #2 > > > > doesn't sound great to me. Since we don't have a way to really separate > out > > > > data: URIs, what we're really talking about is indefinitely allowing > > > Geolocation > > > > on insecure origins for some portion of WebView, which is a pretty hefty > > > > exception. Given that we're pulling the band-aid off for the Web, I don't > > see > > > > why we wouldn't pull the band-aid off for WebView as well. > > > > > > sgurun and rbyers: Thinking about this a little further, how is this > different > > > from any other deprecation and removal we do in Blink? That is, we remove > > > features all of the time, and they could be used in some special way in > > WebView, > > > but we don't have UMA there, so we don't know. That is, we only see this as > a > > > problem because there happened to be a test that leverages this behavior in > a > > > special way. > > > > > > Take, for example, getUserMedia(), which we removed (from insecure origins) > in > > > m47. That had the exact same behavior as here, but we didn't special case it > > in > > > WebView. > > > > > > Or take any of the other recent Blink removals, such as: > > > * getComputedStyle(e).css: > > > > > > https://groups.google.com/a/chromium.org/forum/#!searchin/blink-dev/intent$20... > > > * <applet>: > > > > > > https://groups.google.com/a/chromium.org/forum/#!searchin/blink-dev/intent$20... > > > We didn't special case them for WebView. Is there a reason we should special > > > case geolocation with WebView here? > > > > > > That is to say, I'm of the opinion that it would better to proceed with the > > > deprecation and we can react as necessary. If it turns out to be a huge deal > > for > > > WebView, we can react accordingly, just as we would plan to do for any other > > > platform. > > > > > > Please let me know where I've gone off the deep end here :-) > > > > It is not easy to draw a line. We need to improve security of the platform but > > we also need to keep the platform functioning. I guess you mean reverting or > I completely agree, and it wasn't obvious that we would decide to deprecate and > remove Geolocation. This is why we went through the Blink deprecation and > removal process and the consensus was that the trade-off is worth the functional > cost, which lead to API owner approval. On the other side, fullscreen on > insecure origins was originally slated for deprecation and removal, but we undid > that decision because the numbers were not good and it was not a huge security > win. > > Do you feel that there's a different set of use cases for WebView then the > general Web that warrant leaving a security exception in place or that make the > Blink platform decision incorrect? If so, it would be great to outline those > cases so we can understand them better and hopefully address any problems there > so we can continue to move the Web to a more secure place. > > providing a quirk mode if it turns out to be a great deal. > > > > I am going to check our compatibility test suite (which is a test suite that > > guarantees api behavior) tomorrow to see what behavior we are testing there. > Cool, thanks! torne, I will look at this again today, but your comments are welcome :) one problem is I do not have any data to tell me what will be broken so I naturally tend for the safer option.
I think the major special thing about WebView here is that lots of WebView apps seem to use data: URIs to load a piece of content into the WebView that the app either downloaded itself using a separate HTTP stack (the Android one, or another HTTP stack in the app), or shipped inside its APK. They often consider this content trusted, though sometimes they may not actually be correct about this. I wonder if there's any way we could (or should) consider URLs loaded by WebView.loadUrl/loadData/loadDataWithBaseURL as "more trustworthy" than URLs reached through navigations? That might be a mitigation, though I'm not sure it's a good idea. A secondary problem is that many Android app developers haven't yet adjusted to the world where WebView is regularly updated, and still expect that backward incompatible changes will be rare (as they are for the Android OS APIs) and potentially years apart. When Android deprecates an API, it's often deprecated for well over a year (one or more full major OS releases), and even after that deprecation period, is often *only* removed from the SDK (preventing apps from being compiled if they use it) and is *not* actually removed from the OS, ensuring that existing apps continue to function, or is only disabled at runtime for apps that target a new OS API level and not really removed at all. I understand that the web can't possibly work that way, and that WebView ultimately *is* the web, but this still surprises many app developers and their immediate reaction to backward incompatible changes so far has basically *always* been "so how long until you release a version that undoes this change and fixes my app" - they don't always have the same mindset as developers who target the web in general. Because we don't have UMA stats for WebView it's hard to get a grasp on the possible scope of the problem. The beta channel helps a bit, but the WebView beta channel has on the general order of 10K users and a nontrivial proportion of them are not "regular" users but test accounts used by app developers, Android device OEMs, etc - who will probably spot issues in the applications they personally are responsible for, but don't test a representative sample of applications. Past issues with web features removed after deprecation (e.g. the overflowchanged event) have not really been uncovered until several weeks *after* the stable release has already been pushed to 100% of users, as that's how long it takes for a developer to get enough reports of breakage, investigate them, figure out it's a webview issue, file a chromium bug (which rarely actually has a diagnosis, just "my app used to work and now doesn't"), and for us to figure out that it's caused by the removal of a deprecated feature. The evidence suggests that most webview developers don't use the chrome devtools and probably don't see console deprecation warnings in many cases. Initially it was also a problem that only Android 5.0+ devices got WebView updates, since this meant that many developers did not develop or test on a device that even got updates at all, but hopefully this is changing now that ~30% of the ecosystem is running 5.0 and up. I'm not sure what the conclusion should be about geolocation, really; I'll see what else I can think of that's more specific to this, but just wanted to share some general thoughts I've already had. I realise this is a bit of a depressing picture, and I apologise in advance to any Android app developers who read this - I'm sure *you* are a good developer. ;)
Thanks, Torne. Special casing data: URIs as secure when loaded via loadUrl/loadData sounds reasonable to me from a security perspective, but we've been chatting about that offline, and we're really not sure how that would work. It would require a level of taint tracking on GURLs that I don't think we have the infrastructure for. Do you have insight about how that can be done? Your other points about WebView and updates definitely make sense to me, but I don't see them as particular to this specific case. What they object to are changes, in general, to the Web platform (i.e. Blink), not to this particular case. As I mentioned in an earlier message, the only reason we even noticed this change is because there happened to be a test that relied on this behavior; had this test used loadDataWithBaseUrl (as I've implemented in this CL), it would have simply been another Web platform change that would be upstreamed to WebView. So while I understand the general concern about Web platform changes and WebView (and think that would probably be a good discussion to start on blink-dev), I guess I'm still asking why this *particular* case is different and special compared to other Web platform changes? On 2015/12/10 18:22:58, Torne wrote: > I think the major special thing about WebView here is that lots of WebView apps > seem to use data: URIs to load a piece of content into the WebView that the app > either downloaded itself using a separate HTTP stack (the Android one, or > another HTTP stack in the app), or shipped inside its APK. They often consider > this content trusted, though sometimes they may not actually be correct about > this. I wonder if there's any way we could (or should) consider URLs loaded by > WebView.loadUrl/loadData/loadDataWithBaseURL as "more trustworthy" than URLs > reached through navigations? That might be a mitigation, though I'm not sure > it's a good idea. > > A secondary problem is that many Android app developers haven't yet adjusted to > the world where WebView is regularly updated, and still expect that backward > incompatible changes will be rare (as they are for the Android OS APIs) and > potentially years apart. When Android deprecates an API, it's often deprecated > for well over a year (one or more full major OS releases), and even after that > deprecation period, is often *only* removed from the SDK (preventing apps from > being compiled if they use it) and is *not* actually removed from the OS, > ensuring that existing apps continue to function, or is only disabled at runtime > for apps that target a new OS API level and not really removed at all. I > understand that the web can't possibly work that way, and that WebView > ultimately *is* the web, but this still surprises many app developers and their > immediate reaction to backward incompatible changes so far has basically > *always* been "so how long until you release a version that undoes this change > and fixes my app" - they don't always have the same mindset as developers who > target the web in general. > > Because we don't have UMA stats for WebView it's hard to get a grasp on the > possible scope of the problem. The beta channel helps a bit, but the WebView > beta channel has on the general order of 10K users and a nontrivial proportion > of them are not "regular" users but test accounts used by app developers, > Android device OEMs, etc - who will probably spot issues in the applications > they personally are responsible for, but don't test a representative sample of > applications. > > Past issues with web features removed after deprecation (e.g. the > overflowchanged event) have not really been uncovered until several weeks > *after* the stable release has already been pushed to 100% of users, as that's > how long it takes for a developer to get enough reports of breakage, investigate > them, figure out it's a webview issue, file a chromium bug (which rarely > actually has a diagnosis, just "my app used to work and now doesn't"), and for > us to figure out that it's caused by the removal of a deprecated feature. The > evidence suggests that most webview developers don't use the chrome devtools and > probably don't see console deprecation warnings in many cases. Initially it was > also a problem that only Android 5.0+ devices got WebView updates, since this > meant that many developers did not develop or test on a device that even got > updates at all, but hopefully this is changing now that ~30% of the ecosystem is > running 5.0 and up. > > I'm not sure what the conclusion should be about geolocation, really; I'll see > what else I can think of that's more specific to this, but just wanted to share > some general thoughts I've already had. > > I realise this is a bit of a depressing picture, and I apologise in advance to > any Android app developers who read this - I'm sure *you* are a good developer. > ;)
On 2015/12/10 18:33:20, jww wrote: > Thanks, Torne. Special casing data: URIs as secure when loaded via > loadUrl/loadData sounds reasonable to me from a security perspective, but we've > been chatting about that offline, and we're really not sure how that would work. > It would require a level of taint tracking on GURLs that I don't think we have > the infrastructure for. Do you have insight about how that can be done? > > Your other points about WebView and updates definitely make sense to me, but I > don't see them as particular to this specific case. What they object to are > changes, in general, to the Web platform (i.e. Blink), not to this particular > case. As I mentioned in an earlier message, the only reason we even noticed this > change is because there happened to be a test that relied on this behavior; had > this test used loadDataWithBaseUrl (as I've implemented in this CL), it would > have simply been another Web platform change that would be upstreamed to > WebView. So while I understand the general concern about Web platform changes > and WebView (and think that would probably be a good discussion to start on > blink-dev), I guess I'm still asking why this *particular* case is different and > special compared to other Web platform changes? > > On 2015/12/10 18:22:58, Torne wrote: > > I think the major special thing about WebView here is that lots of WebView > apps > > seem to use data: URIs to load a piece of content into the WebView that the > app > > either downloaded itself using a separate HTTP stack (the Android one, or > > another HTTP stack in the app), or shipped inside its APK. They often consider > > this content trusted, though sometimes they may not actually be correct about > > this. I wonder if there's any way we could (or should) consider URLs loaded by > > WebView.loadUrl/loadData/loadDataWithBaseURL as "more trustworthy" than URLs > > reached through navigations? That might be a mitigation, though I'm not sure > > it's a good idea. > > > > A secondary problem is that many Android app developers haven't yet adjusted > to > > the world where WebView is regularly updated, and still expect that backward > > incompatible changes will be rare (as they are for the Android OS APIs) and > > potentially years apart. When Android deprecates an API, it's often deprecated > > for well over a year (one or more full major OS releases), and even after that > > deprecation period, is often *only* removed from the SDK (preventing apps from > > being compiled if they use it) and is *not* actually removed from the OS, > > ensuring that existing apps continue to function, or is only disabled at > runtime > > for apps that target a new OS API level and not really removed at all. I > > understand that the web can't possibly work that way, and that WebView > > ultimately *is* the web, but this still surprises many app developers and > their > > immediate reaction to backward incompatible changes so far has basically > > *always* been "so how long until you release a version that undoes this change > > and fixes my app" - they don't always have the same mindset as developers who > > target the web in general. > > > > Because we don't have UMA stats for WebView it's hard to get a grasp on the > > possible scope of the problem. The beta channel helps a bit, but the WebView > > beta channel has on the general order of 10K users and a nontrivial proportion > > of them are not "regular" users but test accounts used by app developers, > > Android device OEMs, etc - who will probably spot issues in the applications > > they personally are responsible for, but don't test a representative sample of > > applications. > > > > Past issues with web features removed after deprecation (e.g. the > > overflowchanged event) have not really been uncovered until several weeks > > *after* the stable release has already been pushed to 100% of users, as that's > > how long it takes for a developer to get enough reports of breakage, > investigate > > them, figure out it's a webview issue, file a chromium bug (which rarely > > actually has a diagnosis, just "my app used to work and now doesn't"), and for > > us to figure out that it's caused by the removal of a deprecated feature. The > > evidence suggests that most webview developers don't use the chrome devtools > and > > probably don't see console deprecation warnings in many cases. Initially it > was > > also a problem that only Android 5.0+ devices got WebView updates, since this > > meant that many developers did not develop or test on a device that even got > > updates at all, but hopefully this is changing now that ~30% of the ecosystem > is > > running 5.0 and up. > > > > I'm not sure what the conclusion should be about geolocation, really; I'll see > > what else I can think of that's more specific to this, but just wanted to > share > > some general thoughts I've already had. > > > > I realise this is a bit of a depressing picture, and I apologise in advance to > > any Android app developers who read this - I'm sure *you* are a good > developer. > > ;) Unfortunately our test coverage is not so great. We also have a secondary test suite CTS, which is also covering a bit more, but it is not on main waterfall yet (it is on FYI). We could also discover this problem there since there are some geolocation tests. CTS is the compatibility suite that tests android implementation and guarantees how it would behave across devices. We are working on UMA and increasing test coverage.
On 2015/12/10 18:33:20, jww wrote: > Thanks, Torne. Special casing data: URIs as secure when loaded via > loadUrl/loadData sounds reasonable to me from a security perspective, but we've > been chatting about that offline, and we're really not sure how that would work. > It would require a level of taint tracking on GURLs that I don't think we have > the infrastructure for. Do you have insight about how that can be done? Not really. Probably the ideal would be if WebView could just push people to use loadDataWithBaseURL when they're loading their own content into WebView, giving it a real origin so that this class of problem doesn't exist, but that doesn't solve the problem of maybe breaking existing apps. :/ > Your other points about WebView and updates definitely make sense to me, but I > don't see them as particular to this specific case. What they object to are > changes, in general, to the Web platform (i.e. Blink), not to this particular > case. As I mentioned in an earlier message, the only reason we even noticed this > change is because there happened to be a test that relied on this behavior; had > this test used loadDataWithBaseUrl (as I've implemented in this CL), it would > have simply been another Web platform change that would be upstreamed to > WebView. So while I understand the general concern about Web platform changes > and WebView (and think that would probably be a good discussion to start on > blink-dev), I guess I'm still asking why this *particular* case is different and > special compared to other Web platform changes? Sorry, I didn't mean to imply this is different and special compared to other changes, just giving background on why we worry about things. I think the specific issue here is that it just "seems" like this might be something people are relying on, since we do know that people do weird stuff with data: URIs and expect them to be treated the same as other web content loaded from a regular origin, but I don't think we have any concrete basis to believe that it's common to specifically want geolocation to work in that context. Shipping the change is probably the only way to find out whether our vague feelings are associated with a real issue. I don't have a particularly strong feeling about this either way really.. One thing we could do as a compromise, maybe, is put the change behind a flag for one release (i.e. disable geolocation on http, unless the flag is set), just so that if we discover this causes widespread app breakage, we can flip the flag on for webview in beta/stable to give us another release to decide what to do, without reverting the CL for all platforms, or having to minibranch to revert the CL just for WebView, but it might not be worth the effort of doing that (if we really had to, we could just put it back behind a flag on the branch too).
On 2015/12/10 18:58:13, Torne wrote: > On 2015/12/10 18:33:20, jww wrote: > > Thanks, Torne. Special casing data: URIs as secure when loaded via > > loadUrl/loadData sounds reasonable to me from a security perspective, but > we've > > been chatting about that offline, and we're really not sure how that would > work. > > It would require a level of taint tracking on GURLs that I don't think we have > > the infrastructure for. Do you have insight about how that can be done? > > Not really. Probably the ideal would be if WebView could just push people to use > loadDataWithBaseURL when they're loading their own content into WebView, giving > it a real origin so that this class of problem doesn't exist, but that doesn't > solve the problem of maybe breaking existing apps. :/ > > > Your other points about WebView and updates definitely make sense to me, but I > > don't see them as particular to this specific case. What they object to are > > changes, in general, to the Web platform (i.e. Blink), not to this particular > > case. As I mentioned in an earlier message, the only reason we even noticed > this > > change is because there happened to be a test that relied on this behavior; > had > > this test used loadDataWithBaseUrl (as I've implemented in this CL), it would > > have simply been another Web platform change that would be upstreamed to > > WebView. So while I understand the general concern about Web platform changes > > and WebView (and think that would probably be a good discussion to start on > > blink-dev), I guess I'm still asking why this *particular* case is different > and > > special compared to other Web platform changes? > > Sorry, I didn't mean to imply this is different and special compared to other > changes, just giving background on why we worry about things. I think the > specific issue here is that it just "seems" like this might be something people > are relying on, since we do know that people do weird stuff with data: URIs and > expect them to be treated the same as other web content loaded from a regular > origin, but I don't think we have any concrete basis to believe that it's common > to specifically want geolocation to work in that context. Shipping the change is > probably the only way to find out whether our vague feelings are associated with > a real issue. I don't have a particularly strong feeling about this either way > really.. > > One thing we could do as a compromise, maybe, is put the change behind a flag > for one release (i.e. disable geolocation on http, unless the flag is set), just > so that if we discover this causes widespread app breakage, we can flip the flag > on for webview in beta/stable to give us another release to decide what to do, > without reverting the CL for all platforms, or having to minibranch to revert > the CL just for WebView, but it might not be worth the effort of doing that (if > we really had to, we could just put it back behind a flag on the branch too). Sure, I don't mind making a flag to disable it if it turns out to be a large problem. sgurun, how does that sound to you? Although, to be clear, I fully expect there to be complains from developers (there were for getUserMedia()), so I'd expect we'd only flip that flag on WebView under overwhelming circumstances.
On 2015/12/10 21:41:07, jww wrote: > On 2015/12/10 18:58:13, Torne wrote: > > On 2015/12/10 18:33:20, jww wrote: > > > Thanks, Torne. Special casing data: URIs as secure when loaded via > > > loadUrl/loadData sounds reasonable to me from a security perspective, but > > we've > > > been chatting about that offline, and we're really not sure how that would > > work. > > > It would require a level of taint tracking on GURLs that I don't think we > have > > > the infrastructure for. Do you have insight about how that can be done? > > > > Not really. Probably the ideal would be if WebView could just push people to > use > > loadDataWithBaseURL when they're loading their own content into WebView, > giving > > it a real origin so that this class of problem doesn't exist, but that doesn't > > solve the problem of maybe breaking existing apps. :/ > > > > > Your other points about WebView and updates definitely make sense to me, but > I > > > don't see them as particular to this specific case. What they object to are > > > changes, in general, to the Web platform (i.e. Blink), not to this > particular > > > case. As I mentioned in an earlier message, the only reason we even noticed > > this > > > change is because there happened to be a test that relied on this behavior; > > had > > > this test used loadDataWithBaseUrl (as I've implemented in this CL), it > would > > > have simply been another Web platform change that would be upstreamed to > > > WebView. So while I understand the general concern about Web platform > changes > > > and WebView (and think that would probably be a good discussion to start on > > > blink-dev), I guess I'm still asking why this *particular* case is different > > and > > > special compared to other Web platform changes? > > > > Sorry, I didn't mean to imply this is different and special compared to other > > changes, just giving background on why we worry about things. I think the > > specific issue here is that it just "seems" like this might be something > people > > are relying on, since we do know that people do weird stuff with data: URIs > and > > expect them to be treated the same as other web content loaded from a regular > > origin, but I don't think we have any concrete basis to believe that it's > common > > to specifically want geolocation to work in that context. Shipping the change > is > > probably the only way to find out whether our vague feelings are associated > with > > a real issue. I don't have a particularly strong feeling about this either way > > really.. > > > > One thing we could do as a compromise, maybe, is put the change behind a flag > > for one release (i.e. disable geolocation on http, unless the flag is set), > just > > so that if we discover this causes widespread app breakage, we can flip the > flag > > on for webview in beta/stable to give us another release to decide what to do, > > without reverting the CL for all platforms, or having to minibranch to revert > > the CL just for WebView, but it might not be worth the effort of doing that > (if > > we really had to, we could just put it back behind a flag on the branch too). > > Sure, I don't mind making a flag to disable it if it turns out to be a large > problem. sgurun, how does that sound to you? Although, to be clear, I fully > expect there to be complains from developers (there were for getUserMedia()), so > I'd expect we'd only flip that flag on WebView under overwhelming circumstances. lgtm
On 2015/12/10 21:52:38, sgurun wrote: > On 2015/12/10 21:41:07, jww wrote: > > On 2015/12/10 18:58:13, Torne wrote: > > > On 2015/12/10 18:33:20, jww wrote: > > > > Thanks, Torne. Special casing data: URIs as secure when loaded via > > > > loadUrl/loadData sounds reasonable to me from a security perspective, but > > > we've > > > > been chatting about that offline, and we're really not sure how that would > > > work. > > > > It would require a level of taint tracking on GURLs that I don't think we > > have > > > > the infrastructure for. Do you have insight about how that can be done? > > > > > > Not really. Probably the ideal would be if WebView could just push people to > > use > > > loadDataWithBaseURL when they're loading their own content into WebView, > > giving > > > it a real origin so that this class of problem doesn't exist, but that > doesn't > > > solve the problem of maybe breaking existing apps. :/ > > > > > > > Your other points about WebView and updates definitely make sense to me, > but > > I > > > > don't see them as particular to this specific case. What they object to > are > > > > changes, in general, to the Web platform (i.e. Blink), not to this > > particular > > > > case. As I mentioned in an earlier message, the only reason we even > noticed > > > this > > > > change is because there happened to be a test that relied on this > behavior; > > > had > > > > this test used loadDataWithBaseUrl (as I've implemented in this CL), it > > would > > > > have simply been another Web platform change that would be upstreamed to > > > > WebView. So while I understand the general concern about Web platform > > changes > > > > and WebView (and think that would probably be a good discussion to start > on > > > > blink-dev), I guess I'm still asking why this *particular* case is > different > > > and > > > > special compared to other Web platform changes? > > > > > > Sorry, I didn't mean to imply this is different and special compared to > other > > > changes, just giving background on why we worry about things. I think the > > > specific issue here is that it just "seems" like this might be something > > people > > > are relying on, since we do know that people do weird stuff with data: URIs > > and > > > expect them to be treated the same as other web content loaded from a > regular > > > origin, but I don't think we have any concrete basis to believe that it's > > common > > > to specifically want geolocation to work in that context. Shipping the > change > > is > > > probably the only way to find out whether our vague feelings are associated > > with > > > a real issue. I don't have a particularly strong feeling about this either > way > > > really.. > > > > > > One thing we could do as a compromise, maybe, is put the change behind a > flag > > > for one release (i.e. disable geolocation on http, unless the flag is set), > > just > > > so that if we discover this causes widespread app breakage, we can flip the > > flag > > > on for webview in beta/stable to give us another release to decide what to > do, > > > without reverting the CL for all platforms, or having to minibranch to > revert > > > the CL just for WebView, but it might not be worth the effort of doing that > > (if > > > we really had to, we could just put it back behind a flag on the branch > too). > > > > Sure, I don't mind making a flag to disable it if it turns out to be a large > > problem. sgurun, how does that sound to you? Although, to be clear, I fully > > expect there to be complains from developers (there were for getUserMedia()), > so > > I'd expect we'd only flip that flag on WebView under overwhelming > circumstances. > > lgtm Great. To clarify, if I implement chrome://flags option and a command line flag, that will give you (WebView) the flexibility to flip that flag if needed?
On 2015/12/10 22:09:18, jww wrote: > On 2015/12/10 21:52:38, sgurun wrote: > > On 2015/12/10 21:41:07, jww wrote: > > > On 2015/12/10 18:58:13, Torne wrote: > > > > On 2015/12/10 18:33:20, jww wrote: > > > > > Thanks, Torne. Special casing data: URIs as secure when loaded via > > > > > loadUrl/loadData sounds reasonable to me from a security perspective, > but > > > > we've > > > > > been chatting about that offline, and we're really not sure how that > would > > > > work. > > > > > It would require a level of taint tracking on GURLs that I don't think > we > > > have > > > > > the infrastructure for. Do you have insight about how that can be done? > > > > > > > > Not really. Probably the ideal would be if WebView could just push people > to > > > use > > > > loadDataWithBaseURL when they're loading their own content into WebView, > > > giving > > > > it a real origin so that this class of problem doesn't exist, but that > > doesn't > > > > solve the problem of maybe breaking existing apps. :/ > > > > > > > > > Your other points about WebView and updates definitely make sense to me, > > but > > > I > > > > > don't see them as particular to this specific case. What they object to > > are > > > > > changes, in general, to the Web platform (i.e. Blink), not to this > > > particular > > > > > case. As I mentioned in an earlier message, the only reason we even > > noticed > > > > this > > > > > change is because there happened to be a test that relied on this > > behavior; > > > > had > > > > > this test used loadDataWithBaseUrl (as I've implemented in this CL), it > > > would > > > > > have simply been another Web platform change that would be upstreamed to > > > > > WebView. So while I understand the general concern about Web platform > > > changes > > > > > and WebView (and think that would probably be a good discussion to start > > on > > > > > blink-dev), I guess I'm still asking why this *particular* case is > > different > > > > and > > > > > special compared to other Web platform changes? > > > > > > > > Sorry, I didn't mean to imply this is different and special compared to > > other > > > > changes, just giving background on why we worry about things. I think the > > > > specific issue here is that it just "seems" like this might be something > > > people > > > > are relying on, since we do know that people do weird stuff with data: > URIs > > > and > > > > expect them to be treated the same as other web content loaded from a > > regular > > > > origin, but I don't think we have any concrete basis to believe that it's > > > common > > > > to specifically want geolocation to work in that context. Shipping the > > change > > > is > > > > probably the only way to find out whether our vague feelings are > associated > > > with > > > > a real issue. I don't have a particularly strong feeling about this either > > way > > > > really.. > > > > > > > > One thing we could do as a compromise, maybe, is put the change behind a > > flag > > > > for one release (i.e. disable geolocation on http, unless the flag is > set), > > > just > > > > so that if we discover this causes widespread app breakage, we can flip > the > > > flag > > > > on for webview in beta/stable to give us another release to decide what to > > do, > > > > without reverting the CL for all platforms, or having to minibranch to > > revert > > > > the CL just for WebView, but it might not be worth the effort of doing > that > > > (if > > > > we really had to, we could just put it back behind a flag on the branch > > too). > > > > > > Sure, I don't mind making a flag to disable it if it turns out to be a large > > > problem. sgurun, how does that sound to you? Although, to be clear, I fully > > > expect there to be complains from developers (there were for > getUserMedia()), > > so > > > I'd expect we'd only flip that flag on WebView under overwhelming > > circumstances. > > > > lgtm > > Great. To clarify, if I implement chrome://flags option and a command line flag, > that will give you (WebView) the flexibility to flip that flag if needed? webview depends on content. a command line flag that is living at content layer, an API in content, or even a home in WebPreferences are a few things that comes to my mind.
On 2015/12/10 22:29:50, sgurun wrote: > On 2015/12/10 22:09:18, jww wrote: > > On 2015/12/10 21:52:38, sgurun wrote: > > > On 2015/12/10 21:41:07, jww wrote: > > > > On 2015/12/10 18:58:13, Torne wrote: > > > > > On 2015/12/10 18:33:20, jww wrote: > > > > > > Thanks, Torne. Special casing data: URIs as secure when loaded via > > > > > > loadUrl/loadData sounds reasonable to me from a security perspective, > > but > > > > > we've > > > > > > been chatting about that offline, and we're really not sure how that > > would > > > > > work. > > > > > > It would require a level of taint tracking on GURLs that I don't think > > we > > > > have > > > > > > the infrastructure for. Do you have insight about how that can be > done? > > > > > > > > > > Not really. Probably the ideal would be if WebView could just push > people > > to > > > > use > > > > > loadDataWithBaseURL when they're loading their own content into WebView, > > > > giving > > > > > it a real origin so that this class of problem doesn't exist, but that > > > doesn't > > > > > solve the problem of maybe breaking existing apps. :/ > > > > > > > > > > > Your other points about WebView and updates definitely make sense to > me, > > > but > > > > I > > > > > > don't see them as particular to this specific case. What they object > to > > > are > > > > > > changes, in general, to the Web platform (i.e. Blink), not to this > > > > particular > > > > > > case. As I mentioned in an earlier message, the only reason we even > > > noticed > > > > > this > > > > > > change is because there happened to be a test that relied on this > > > behavior; > > > > > had > > > > > > this test used loadDataWithBaseUrl (as I've implemented in this CL), > it > > > > would > > > > > > have simply been another Web platform change that would be upstreamed > to > > > > > > WebView. So while I understand the general concern about Web platform > > > > changes > > > > > > and WebView (and think that would probably be a good discussion to > start > > > on > > > > > > blink-dev), I guess I'm still asking why this *particular* case is > > > different > > > > > and > > > > > > special compared to other Web platform changes? > > > > > > > > > > Sorry, I didn't mean to imply this is different and special compared to > > > other > > > > > changes, just giving background on why we worry about things. I think > the > > > > > specific issue here is that it just "seems" like this might be something > > > > people > > > > > are relying on, since we do know that people do weird stuff with data: > > URIs > > > > and > > > > > expect them to be treated the same as other web content loaded from a > > > regular > > > > > origin, but I don't think we have any concrete basis to believe that > it's > > > > common > > > > > to specifically want geolocation to work in that context. Shipping the > > > change > > > > is > > > > > probably the only way to find out whether our vague feelings are > > associated > > > > with > > > > > a real issue. I don't have a particularly strong feeling about this > either > > > way > > > > > really.. > > > > > > > > > > One thing we could do as a compromise, maybe, is put the change behind a > > > flag > > > > > for one release (i.e. disable geolocation on http, unless the flag is > > set), > > > > just > > > > > so that if we discover this causes widespread app breakage, we can flip > > the > > > > flag > > > > > on for webview in beta/stable to give us another release to decide what > to > > > do, > > > > > without reverting the CL for all platforms, or having to minibranch to > > > revert > > > > > the CL just for WebView, but it might not be worth the effort of doing > > that > > > > (if > > > > > we really had to, we could just put it back behind a flag on the branch > > > too). > > > > > > > > Sure, I don't mind making a flag to disable it if it turns out to be a > large > > > > problem. sgurun, how does that sound to you? Although, to be clear, I > fully > > > > expect there to be complains from developers (there were for > > getUserMedia()), > > > so > > > > I'd expect we'd only flip that flag on WebView under overwhelming > > > circumstances. > > > > > > lgtm > > > > Great. To clarify, if I implement chrome://flags option and a command line > flag, > > that will give you (WebView) the flexibility to flip that flag if needed? > > webview depends on content. a command line flag that is living at content layer, > an API in content, or even a home in WebPreferences are a few things that comes > to my mind. Okay. In that case, I'll commit this, and then I'll write up a new CL to add a command line flag, and I'll make sure to add you as a reviewer to make sure it will work for WebView. Thanks for your help on this!
The CQ bit was checked by jww@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from thestig@chromium.org, philipj@opera.com, mlamouri@chromium.org Link to the patchset: https://codereview.chromium.org/1485973002/#ps100001 (title: "Fix WebView tests")
On 2015/12/10 23:55:57, jww wrote: > On 2015/12/10 22:29:50, sgurun wrote: > > On 2015/12/10 22:09:18, jww wrote: > > > On 2015/12/10 21:52:38, sgurun wrote: > > > > On 2015/12/10 21:41:07, jww wrote: > > > > > On 2015/12/10 18:58:13, Torne wrote: > > > > > > On 2015/12/10 18:33:20, jww wrote: > > > > > > > Thanks, Torne. Special casing data: URIs as secure when loaded via > > > > > > > loadUrl/loadData sounds reasonable to me from a security > perspective, > > > but > > > > > > we've > > > > > > > been chatting about that offline, and we're really not sure how that > > > would > > > > > > work. > > > > > > > It would require a level of taint tracking on GURLs that I don't > think > > > we > > > > > have > > > > > > > the infrastructure for. Do you have insight about how that can be > > done? > > > > > > > > > > > > Not really. Probably the ideal would be if WebView could just push > > people > > > to > > > > > use > > > > > > loadDataWithBaseURL when they're loading their own content into > WebView, > > > > > giving > > > > > > it a real origin so that this class of problem doesn't exist, but that > > > > doesn't > > > > > > solve the problem of maybe breaking existing apps. :/ > > > > > > > > > > > > > Your other points about WebView and updates definitely make sense to > > me, > > > > but > > > > > I > > > > > > > don't see them as particular to this specific case. What they object > > to > > > > are > > > > > > > changes, in general, to the Web platform (i.e. Blink), not to this > > > > > particular > > > > > > > case. As I mentioned in an earlier message, the only reason we even > > > > noticed > > > > > > this > > > > > > > change is because there happened to be a test that relied on this > > > > behavior; > > > > > > had > > > > > > > this test used loadDataWithBaseUrl (as I've implemented in this CL), > > it > > > > > would > > > > > > > have simply been another Web platform change that would be > upstreamed > > to > > > > > > > WebView. So while I understand the general concern about Web > platform > > > > > changes > > > > > > > and WebView (and think that would probably be a good discussion to > > start > > > > on > > > > > > > blink-dev), I guess I'm still asking why this *particular* case is > > > > different > > > > > > and > > > > > > > special compared to other Web platform changes? > > > > > > > > > > > > Sorry, I didn't mean to imply this is different and special compared > to > > > > other > > > > > > changes, just giving background on why we worry about things. I think > > the > > > > > > specific issue here is that it just "seems" like this might be > something > > > > > people > > > > > > are relying on, since we do know that people do weird stuff with data: > > > URIs > > > > > and > > > > > > expect them to be treated the same as other web content loaded from a > > > > regular > > > > > > origin, but I don't think we have any concrete basis to believe that > > it's > > > > > common > > > > > > to specifically want geolocation to work in that context. Shipping the > > > > change > > > > > is > > > > > > probably the only way to find out whether our vague feelings are > > > associated > > > > > with > > > > > > a real issue. I don't have a particularly strong feeling about this > > either > > > > way > > > > > > really.. > > > > > > > > > > > > One thing we could do as a compromise, maybe, is put the change behind > a > > > > flag > > > > > > for one release (i.e. disable geolocation on http, unless the flag is > > > set), > > > > > just > > > > > > so that if we discover this causes widespread app breakage, we can > flip > > > the > > > > > flag > > > > > > on for webview in beta/stable to give us another release to decide > what > > to > > > > do, > > > > > > without reverting the CL for all platforms, or having to minibranch to > > > > revert > > > > > > the CL just for WebView, but it might not be worth the effort of doing > > > that > > > > > (if > > > > > > we really had to, we could just put it back behind a flag on the > branch > > > > too). > > > > > > > > > > Sure, I don't mind making a flag to disable it if it turns out to be a > > large > > > > > problem. sgurun, how does that sound to you? Although, to be clear, I > > fully > > > > > expect there to be complains from developers (there were for > > > getUserMedia()), > > > > so > > > > > I'd expect we'd only flip that flag on WebView under overwhelming > > > > circumstances. > > > > > > > > lgtm > > > > > > Great. To clarify, if I implement chrome://flags option and a command line > > flag, > > > that will give you (WebView) the flexibility to flip that flag if needed? > > > > webview depends on content. a command line flag that is living at content > layer, > > an API in content, or even a home in WebPreferences are a few things that > comes > > to my mind. > > Okay. In that case, I'll commit this, and then I'll write up a new CL to add a > command line flag, and I'll make sure to add you as a reviewer to make sure it > will work for WebView. Thanks for your help on this! thanks
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1485973002/100001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1485973002/100001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_android_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_android_r...)
The CQ bit was checked by jww@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1485973002/100001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1485973002/100001
Message was sent while issue was closed.
Description was changed from ========== Removal of geolocation APIs on insecure origins This disallows the geolocation APIs getCurrentPosition() and watchPosition() from being used on insecure origins. Adds a console warning message that the API call has failed because of this. BUG=520765,561641 ========== to ========== Removal of geolocation APIs on insecure origins This disallows the geolocation APIs getCurrentPosition() and watchPosition() from being used on insecure origins. Adds a console warning message that the API call has failed because of this. BUG=520765,561641 ==========
Message was sent while issue was closed.
Committed patchset #6 (id:100001)
Message was sent while issue was closed.
Description was changed from ========== Removal of geolocation APIs on insecure origins This disallows the geolocation APIs getCurrentPosition() and watchPosition() from being used on insecure origins. Adds a console warning message that the API call has failed because of this. BUG=520765,561641 ========== to ========== Removal of geolocation APIs on insecure origins This disallows the geolocation APIs getCurrentPosition() and watchPosition() from being used on insecure origins. Adds a console warning message that the API call has failed because of this. BUG=520765,561641 Committed: https://crrev.com/33ef9f5c8df422b0320cbc506d57bdce2999ebc8 Cr-Commit-Position: refs/heads/master@{#364642} ==========
Message was sent while issue was closed.
Patchset 6 (id:??) landed as https://crrev.com/33ef9f5c8df422b0320cbc506d57bdce2999ebc8 Cr-Commit-Position: refs/heads/master@{#364642}
Message was sent while issue was closed.
A revert of this CL (patchset #6 id:100001) has been created in https://codereview.chromium.org/1515103003/ by johnme@chromium.org. The reason for reverting is: Sorry, this broke the following WebView CTS tests: android.webkit.cts.GeolocationTest#testSimpleGeolocationRequestAcceptAlways android.webkit.cts.GeolocationTest#testSimpleGeolocationRequestAcceptOnce android.webkit.cts.GeolocationTest#testSimpleGeolocationRequestReject See https://build.chromium.org/p/chromium.android/builders/Android%20WebView%20CT... It seems that might be intentional, but turning the bot red doesn't seem great. sgurun@ can probably advise on whether WebView has test expectations for CTS, that could be used to disable these tests..
Message was sent while issue was closed.
On 2015/12/10 23:55:57, jww wrote: > On 2015/12/10 22:29:50, sgurun wrote: > > On 2015/12/10 22:09:18, jww wrote: > > > On 2015/12/10 21:52:38, sgurun wrote: > > > > On 2015/12/10 21:41:07, jww wrote: > > > > > On 2015/12/10 18:58:13, Torne wrote: > > > > > > On 2015/12/10 18:33:20, jww wrote: > > > > > > > Thanks, Torne. Special casing data: URIs as secure when loaded via > > > > > > > loadUrl/loadData sounds reasonable to me from a security > perspective, > > > but > > > > > > we've > > > > > > > been chatting about that offline, and we're really not sure how that > > > would > > > > > > work. > > > > > > > It would require a level of taint tracking on GURLs that I don't > think > > > we > > > > > have > > > > > > > the infrastructure for. Do you have insight about how that can be > > done? > > > > > > > > > > > > Not really. Probably the ideal would be if WebView could just push > > people > > > to > > > > > use > > > > > > loadDataWithBaseURL when they're loading their own content into > WebView, > > > > > giving > > > > > > it a real origin so that this class of problem doesn't exist, but that > > > > doesn't > > > > > > solve the problem of maybe breaking existing apps. :/ > > > > > > > > > > > > > Your other points about WebView and updates definitely make sense to > > me, > > > > but > > > > > I > > > > > > > don't see them as particular to this specific case. What they object > > to > > > > are > > > > > > > changes, in general, to the Web platform (i.e. Blink), not to this > > > > > particular > > > > > > > case. As I mentioned in an earlier message, the only reason we even > > > > noticed > > > > > > this > > > > > > > change is because there happened to be a test that relied on this > > > > behavior; > > > > > > had > > > > > > > this test used loadDataWithBaseUrl (as I've implemented in this CL), > > it > > > > > would > > > > > > > have simply been another Web platform change that would be > upstreamed > > to > > > > > > > WebView. So while I understand the general concern about Web > platform > > > > > changes > > > > > > > and WebView (and think that would probably be a good discussion to > > start > > > > on > > > > > > > blink-dev), I guess I'm still asking why this *particular* case is > > > > different > > > > > > and > > > > > > > special compared to other Web platform changes? > > > > > > > > > > > > Sorry, I didn't mean to imply this is different and special compared > to > > > > other > > > > > > changes, just giving background on why we worry about things. I think > > the > > > > > > specific issue here is that it just "seems" like this might be > something > > > > > people > > > > > > are relying on, since we do know that people do weird stuff with data: > > > URIs > > > > > and > > > > > > expect them to be treated the same as other web content loaded from a > > > > regular > > > > > > origin, but I don't think we have any concrete basis to believe that > > it's > > > > > common > > > > > > to specifically want geolocation to work in that context. Shipping the > > > > change > > > > > is > > > > > > probably the only way to find out whether our vague feelings are > > > associated > > > > > with > > > > > > a real issue. I don't have a particularly strong feeling about this > > either > > > > way > > > > > > really.. > > > > > > > > > > > > One thing we could do as a compromise, maybe, is put the change behind > a > > > > flag > > > > > > for one release (i.e. disable geolocation on http, unless the flag is > > > set), > > > > > just > > > > > > so that if we discover this causes widespread app breakage, we can > flip > > > the > > > > > flag > > > > > > on for webview in beta/stable to give us another release to decide > what > > to > > > > do, > > > > > > without reverting the CL for all platforms, or having to minibranch to > > > > revert > > > > > > the CL just for WebView, but it might not be worth the effort of doing > > > that > > > > > (if > > > > > > we really had to, we could just put it back behind a flag on the > branch > > > > too). > > > > > > > > > > Sure, I don't mind making a flag to disable it if it turns out to be a > > large > > > > > problem. sgurun, how does that sound to you? Although, to be clear, I > > fully > > > > > expect there to be complains from developers (there were for > > > getUserMedia()), > > > > so > > > > > I'd expect we'd only flip that flag on WebView under overwhelming > > > > circumstances. > > > > > > > > lgtm > > > > > > Great. To clarify, if I implement chrome://flags option and a command line > > flag, > > > that will give you (WebView) the flexibility to flip that flag if needed? > > > > webview depends on content. a command line flag that is living at content > layer, > > an API in content, or even a home in WebPreferences are a few things that > comes > > to my mind. > > Okay. In that case, I'll commit this, and then I'll write up a new CL to add a > command line flag, and I'll make sure to add you as a reviewer to make sure it > will work for WebView. Thanks for your help on this! Don't add it to chrome://flags, because that's not available on WebView anyway and will signpost it for people on other platforms as something to maybe flip :) We can set arbitrary command line flags programmatically, or call APIs in content if you don't want to expose it as a command line flag at all.
Message was sent while issue was closed.
On 2015/12/11 14:33:24, Torne wrote: > On 2015/12/10 23:55:57, jww wrote: > > On 2015/12/10 22:29:50, sgurun wrote: > > > On 2015/12/10 22:09:18, jww wrote: > > > > On 2015/12/10 21:52:38, sgurun wrote: > > > > > On 2015/12/10 21:41:07, jww wrote: > > > > > > On 2015/12/10 18:58:13, Torne wrote: > > > > > > > On 2015/12/10 18:33:20, jww wrote: > > > > > > > > Thanks, Torne. Special casing data: URIs as secure when loaded via > > > > > > > > loadUrl/loadData sounds reasonable to me from a security > > perspective, > > > > but > > > > > > > we've > > > > > > > > been chatting about that offline, and we're really not sure how > that > > > > would > > > > > > > work. > > > > > > > > It would require a level of taint tracking on GURLs that I don't > > think > > > > we > > > > > > have > > > > > > > > the infrastructure for. Do you have insight about how that can be > > > done? > > > > > > > > > > > > > > Not really. Probably the ideal would be if WebView could just push > > > people > > > > to > > > > > > use > > > > > > > loadDataWithBaseURL when they're loading their own content into > > WebView, > > > > > > giving > > > > > > > it a real origin so that this class of problem doesn't exist, but > that > > > > > doesn't > > > > > > > solve the problem of maybe breaking existing apps. :/ > > > > > > > > > > > > > > > Your other points about WebView and updates definitely make sense > to > > > me, > > > > > but > > > > > > I > > > > > > > > don't see them as particular to this specific case. What they > object > > > to > > > > > are > > > > > > > > changes, in general, to the Web platform (i.e. Blink), not to this > > > > > > particular > > > > > > > > case. As I mentioned in an earlier message, the only reason we > even > > > > > noticed > > > > > > > this > > > > > > > > change is because there happened to be a test that relied on this > > > > > behavior; > > > > > > > had > > > > > > > > this test used loadDataWithBaseUrl (as I've implemented in this > CL), > > > it > > > > > > would > > > > > > > > have simply been another Web platform change that would be > > upstreamed > > > to > > > > > > > > WebView. So while I understand the general concern about Web > > platform > > > > > > changes > > > > > > > > and WebView (and think that would probably be a good discussion to > > > start > > > > > on > > > > > > > > blink-dev), I guess I'm still asking why this *particular* case is > > > > > different > > > > > > > and > > > > > > > > special compared to other Web platform changes? > > > > > > > > > > > > > > Sorry, I didn't mean to imply this is different and special compared > > to > > > > > other > > > > > > > changes, just giving background on why we worry about things. I > think > > > the > > > > > > > specific issue here is that it just "seems" like this might be > > something > > > > > > people > > > > > > > are relying on, since we do know that people do weird stuff with > data: > > > > URIs > > > > > > and > > > > > > > expect them to be treated the same as other web content loaded from > a > > > > > regular > > > > > > > origin, but I don't think we have any concrete basis to believe that > > > it's > > > > > > common > > > > > > > to specifically want geolocation to work in that context. Shipping > the > > > > > change > > > > > > is > > > > > > > probably the only way to find out whether our vague feelings are > > > > associated > > > > > > with > > > > > > > a real issue. I don't have a particularly strong feeling about this > > > either > > > > > way > > > > > > > really.. > > > > > > > > > > > > > > One thing we could do as a compromise, maybe, is put the change > behind > > a > > > > > flag > > > > > > > for one release (i.e. disable geolocation on http, unless the flag > is > > > > set), > > > > > > just > > > > > > > so that if we discover this causes widespread app breakage, we can > > flip > > > > the > > > > > > flag > > > > > > > on for webview in beta/stable to give us another release to decide > > what > > > to > > > > > do, > > > > > > > without reverting the CL for all platforms, or having to minibranch > to > > > > > revert > > > > > > > the CL just for WebView, but it might not be worth the effort of > doing > > > > that > > > > > > (if > > > > > > > we really had to, we could just put it back behind a flag on the > > branch > > > > > too). > > > > > > > > > > > > Sure, I don't mind making a flag to disable it if it turns out to be a > > > large > > > > > > problem. sgurun, how does that sound to you? Although, to be clear, I > > > fully > > > > > > expect there to be complains from developers (there were for > > > > getUserMedia()), > > > > > so > > > > > > I'd expect we'd only flip that flag on WebView under overwhelming > > > > > circumstances. > > > > > > > > > > lgtm > > > > > > > > Great. To clarify, if I implement chrome://flags option and a command line > > > flag, > > > > that will give you (WebView) the flexibility to flip that flag if needed? > > > > > > webview depends on content. a command line flag that is living at content > > layer, > > > an API in content, or even a home in WebPreferences are a few things that > > comes > > > to my mind. > > > > Okay. In that case, I'll commit this, and then I'll write up a new CL to add a > > command line flag, and I'll make sure to add you as a reviewer to make sure it > > will work for WebView. Thanks for your help on this! > > Don't add it to chrome://flags, because that's not available on WebView anyway > and will signpost it for people on other platforms as something to maybe flip :) > We can set arbitrary command line flags programmatically, or call APIs in > content if you don't want to expose it as a command line flag at all. Since 3 CTS tests have failed, I think I change my position on that. These tests live in Android source tree and verify the implementation of API. As Torne mentioned above, changes to Android APIs are done in major Android releases such as KitKat, L, etc and then the partners releasing the devices need to run these tests to make sure they are compatible with Android. CTS tests are released for each of them. However with updatable Webview things are complicated. Devices will start failing the test after this change. Further developers can look at these tests as a guidance while writing their code. If you want ping me and let's have a meeting to discuss more. However the best approach is adding a mechanism to selectively enforce that. Webview can enforce it starting with next major android release.
Message was sent while issue was closed.
On 2015/12/11 14:33:24, Torne wrote: > On 2015/12/10 23:55:57, jww wrote: > > On 2015/12/10 22:29:50, sgurun wrote: > > > On 2015/12/10 22:09:18, jww wrote: > > > > On 2015/12/10 21:52:38, sgurun wrote: > > > > > On 2015/12/10 21:41:07, jww wrote: > > > > > > On 2015/12/10 18:58:13, Torne wrote: > > > > > > > On 2015/12/10 18:33:20, jww wrote: > > > > > > > > Thanks, Torne. Special casing data: URIs as secure when loaded via > > > > > > > > loadUrl/loadData sounds reasonable to me from a security > > perspective, > > > > but > > > > > > > we've > > > > > > > > been chatting about that offline, and we're really not sure how > that > > > > would > > > > > > > work. > > > > > > > > It would require a level of taint tracking on GURLs that I don't > > think > > > > we > > > > > > have > > > > > > > > the infrastructure for. Do you have insight about how that can be > > > done? > > > > > > > > > > > > > > Not really. Probably the ideal would be if WebView could just push > > > people > > > > to > > > > > > use > > > > > > > loadDataWithBaseURL when they're loading their own content into > > WebView, > > > > > > giving > > > > > > > it a real origin so that this class of problem doesn't exist, but > that > > > > > doesn't > > > > > > > solve the problem of maybe breaking existing apps. :/ > > > > > > > > > > > > > > > Your other points about WebView and updates definitely make sense > to > > > me, > > > > > but > > > > > > I > > > > > > > > don't see them as particular to this specific case. What they > object > > > to > > > > > are > > > > > > > > changes, in general, to the Web platform (i.e. Blink), not to this > > > > > > particular > > > > > > > > case. As I mentioned in an earlier message, the only reason we > even > > > > > noticed > > > > > > > this > > > > > > > > change is because there happened to be a test that relied on this > > > > > behavior; > > > > > > > had > > > > > > > > this test used loadDataWithBaseUrl (as I've implemented in this > CL), > > > it > > > > > > would > > > > > > > > have simply been another Web platform change that would be > > upstreamed > > > to > > > > > > > > WebView. So while I understand the general concern about Web > > platform > > > > > > changes > > > > > > > > and WebView (and think that would probably be a good discussion to > > > start > > > > > on > > > > > > > > blink-dev), I guess I'm still asking why this *particular* case is > > > > > different > > > > > > > and > > > > > > > > special compared to other Web platform changes? > > > > > > > > > > > > > > Sorry, I didn't mean to imply this is different and special compared > > to > > > > > other > > > > > > > changes, just giving background on why we worry about things. I > think > > > the > > > > > > > specific issue here is that it just "seems" like this might be > > something > > > > > > people > > > > > > > are relying on, since we do know that people do weird stuff with > data: > > > > URIs > > > > > > and > > > > > > > expect them to be treated the same as other web content loaded from > a > > > > > regular > > > > > > > origin, but I don't think we have any concrete basis to believe that > > > it's > > > > > > common > > > > > > > to specifically want geolocation to work in that context. Shipping > the > > > > > change > > > > > > is > > > > > > > probably the only way to find out whether our vague feelings are > > > > associated > > > > > > with > > > > > > > a real issue. I don't have a particularly strong feeling about this > > > either > > > > > way > > > > > > > really.. > > > > > > > > > > > > > > One thing we could do as a compromise, maybe, is put the change > behind > > a > > > > > flag > > > > > > > for one release (i.e. disable geolocation on http, unless the flag > is > > > > set), > > > > > > just > > > > > > > so that if we discover this causes widespread app breakage, we can > > flip > > > > the > > > > > > flag > > > > > > > on for webview in beta/stable to give us another release to decide > > what > > > to > > > > > do, > > > > > > > without reverting the CL for all platforms, or having to minibranch > to > > > > > revert > > > > > > > the CL just for WebView, but it might not be worth the effort of > doing > > > > that > > > > > > (if > > > > > > > we really had to, we could just put it back behind a flag on the > > branch > > > > > too). > > > > > > > > > > > > Sure, I don't mind making a flag to disable it if it turns out to be a > > > large > > > > > > problem. sgurun, how does that sound to you? Although, to be clear, I > > > fully > > > > > > expect there to be complains from developers (there were for > > > > getUserMedia()), > > > > > so > > > > > > I'd expect we'd only flip that flag on WebView under overwhelming > > > > > circumstances. > > > > > > > > > > lgtm > > > > > > > > Great. To clarify, if I implement chrome://flags option and a command line > > > flag, > > > > that will give you (WebView) the flexibility to flip that flag if needed? > > > > > > webview depends on content. a command line flag that is living at content > > layer, > > > an API in content, or even a home in WebPreferences are a few things that > > comes > > > to my mind. > > > > Okay. In that case, I'll commit this, and then I'll write up a new CL to add a > > command line flag, and I'll make sure to add you as a reviewer to make sure it > > will work for WebView. Thanks for your help on this! > > Don't add it to chrome://flags, because that's not available on WebView anyway > and will signpost it for people on other platforms as something to maybe flip :) > We can set arbitrary command line flags programmatically, or call APIs in > content if you don't want to expose it as a command line flag at all. Since 3 CTS tests have failed, I think I change my position on that. These tests live in Android source tree and verify the implementation of API. As Torne mentioned above, changes to Android APIs are done in major Android releases such as KitKat, L, etc and then the partners releasing the devices need to run these tests to make sure they are compatible with Android. CTS tests are released for each of them. However with updatable Webview things are complicated. Devices will start failing the test after this change. Further developers can look at these tests as a guidance while writing their code. If you want ping me and let's have a meeting to discuss more. However the best approach is adding a mechanism to selectively enforce that. Webview can enforce it starting with next major android release.
Message was sent while issue was closed.
We can talk to the CTS maintainers about updating the tests to use loadDataWithBaseURL, but we'd have to wait until CTS has been updated for 5.0, 5.1 and 6.0 and all three of those have been released for vendors to use.
Message was sent while issue was closed.
On 2015/12/11 17:07:37, Torne wrote: > We can talk to the CTS maintainers about updating the tests to use > loadDataWithBaseURL, but we'd have to wait until CTS has been updated for 5.0, > 5.1 and 6.0 and all three of those have been released for vendors to use. Hi guys. It seems weird to me that we would leave a security hole in WebView because of tests. Is there no way to ship a new set of tests that would, in this case, actually reduce the number of tests that needed to be passed? That is, we could temporarily remove those tests from CTS, and then add them back in with loadDataWithBaseURL for the next Android release.
Message was sent while issue was closed.
On 2015/12/12 01:46:02, jww wrote: > On 2015/12/11 17:07:37, Torne wrote: > > We can talk to the CTS maintainers about updating the tests to use > > loadDataWithBaseURL, but we'd have to wait until CTS has been updated for 5.0, > > 5.1 and 6.0 and all three of those have been released for vendors to use. > > Hi guys. It seems weird to me that we would leave a security hole in WebView > because of tests. Is there no way to ship a new set of tests that would, in this > case, actually reduce the number of tests that needed to be passed? That is, we > could temporarily remove those tests from CTS, and then add them back in with > loadDataWithBaseURL for the next Android release. these tests are not integration or unit tests. These tests define how devices become compatible with Android platform and released as part of android schedule. It is probably better to meet and talk.
Message was sent while issue was closed.
On 2015/12/12 01:50:31, sgurun wrote: > On 2015/12/12 01:46:02, jww wrote: > > On 2015/12/11 17:07:37, Torne wrote: > > > We can talk to the CTS maintainers about updating the tests to use > > > loadDataWithBaseURL, but we'd have to wait until CTS has been updated for > 5.0, > > > 5.1 and 6.0 and all three of those have been released for vendors to use. > > > > Hi guys. It seems weird to me that we would leave a security hole in WebView > > because of tests. Is there no way to ship a new set of tests that would, in > this > > case, actually reduce the number of tests that needed to be passed? That is, > we > > could temporarily remove those tests from CTS, and then add them back in with > > loadDataWithBaseURL for the next Android release. > > these tests are not integration or unit tests. These tests define how devices > become compatible with Android platform and released as part of android > schedule. It is probably better to meet and talk. I understand, but I'm surprised that there is no ability to release an update that removes the burden for vendors. But as I mentioned offline, I setup a meeting to chat. Happy to hash it out then. |
