New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 750280 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Jul 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug



Sign in to add a comment

whitelisting logic should not check non-ws not-http urls

Project Member Reported by sgu...@chromium.org, Jul 28 2017

Issue description

the safebrowsing whitelisting logic is triggered from resource throttle. normally only certain schemes (http and ws) are safebrowsing checked. However, because of its early location, other schemes were also checked and was actually failing a DCHECK.
7-28 17:55:31.769  5760  5760 F DEBUG   : Build fingerprint: 'google/sailfish/sailfish:OMR1/OPM1.170727.001/4222111:userdebug/dev-keys'
07-28 17:55:31.769  5760  5760 F DEBUG   : Revision: '0'
07-28 17:55:31.769  5760  5760 F DEBUG   : ABI: 'arm64'
07-28 17:55:31.769  5760  5760 F DEBUG   : pid: 29925, tid: 5635, name: Chrome_IOThread  >>> com.yahoo.mobile.client.android.mail <<<
07-28 17:55:31.769  5760  5760 F DEBUG   : signal 6 (SIGABRT), code -6 (SI_TKILL), fault addr --------
07-28 17:55:31.770  5760  5760 F DEBUG   : Abort message: '[FATAL:aw_safe_browsing_whitelist_manager.cc(102)] Check failed: components.size() > 0. 
07-28 17:55:31.770  5760  5760 F DEBUG   : #00 0x000000742547046f /data/app/com.google.android.webview-_nd8cLLlWF_sD6lrEaMKsw==/lib/arm64/libbase.cr.so+0x00000000000e946f
07-28 17:55:31.770  5760  5760 F DEBUG   : #01 0x0000007418b742cf /data/app/com.google.android.webview-_nd8cLLlWF_sD6lrEaMKsw==/lib/arm64/libwebviewchromium.cr.so+0x00000000000f42cf
07-28 17:55:31.770  5760  5760 F DEBUG   : #02 0x0000007418b74077 /data/app/com.google.android.webview-_nd8cLLlWF_sD6lrEaMKsw==/lib/arm64/libwebviewchromium.cr.so+0x00000000000f4077

 
Project Member

Comment 1 by bugdroid1@chromium.org, Jul 29 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6146a7c8cdf03d79a6a0df09c7341a65602c55cf

commit 6146a7c8cdf03d79a6a0df09c7341a65602c55cf
Author: Selim Gurun <sgurun@google.com>
Date: Sat Jul 29 02:31:31 2017

Whitelisting logic should not check non-ws non-http urls

Safebrowsing only works on certain schemes. Further the whitelisting
logic has certain assumptions about the URLs that are checked for
whitelisting. Early out if the url does not have the right scheme.

BUG= 750280 

Change-Id: I44d0e1ee005df43070402ee02617cdc8f35c9014
Reviewed-on: https://chromium-review.googlesource.com/592227
Commit-Queue: Selim Gurun <sgurun@chromium.org>
Reviewed-by: Eric Roman <eroman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#490589}
[modify] https://crrev.com/6146a7c8cdf03d79a6a0df09c7341a65602c55cf/android_webview/browser/aw_safe_browsing_whitelist_manager.cc
[modify] https://crrev.com/6146a7c8cdf03d79a6a0df09c7341a65602c55cf/android_webview/browser/aw_safe_browsing_whitelist_manager_unittest.cc

Comment 2 by sgu...@chromium.org, Jul 29 2017

Labels: Merge-Request-61

Comment 3 by gov...@chromium.org, Jul 30 2017

Labels: OS-Android
Project Member

Comment 4 by sheriffbot@chromium.org, Jul 30 2017

Labels: -Merge-Request-61 Hotlist-Merge-Approved Merge-Approved-61
Your change meets the bar and is auto-approved for M61. Please go ahead and merge the CL to branch 3163 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), ketakid @(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 5 by bugdroid1@chromium.org, Jul 31 2017

Labels: -merge-approved-61 merge-merged-3163
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0aa6d012be78159e54fc454ea00737a44b124a4a

commit 0aa6d012be78159e54fc454ea00737a44b124a4a
Author: Selim Gurun <sgurun@google.com>
Date: Mon Jul 31 16:01:41 2017

Whitelisting logic should not check non-ws non-http urls

Safebrowsing only works on certain schemes. Further the whitelisting
logic has certain assumptions about the URLs that are checked for
whitelisting. Early out if the url does not have the right scheme.

BUG= 750280 
TBR=sgurun@google.com

(cherry picked from commit 6146a7c8cdf03d79a6a0df09c7341a65602c55cf)

Change-Id: I44d0e1ee005df43070402ee02617cdc8f35c9014
Reviewed-on: https://chromium-review.googlesource.com/592227
Commit-Queue: Selim Gurun <sgurun@chromium.org>
Reviewed-by: Eric Roman <eroman@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#490589}
Reviewed-on: https://chromium-review.googlesource.com/594327
Reviewed-by: Selim Gurun <sgurun@chromium.org>
Cr-Commit-Position: refs/branch-heads/3163@{#157}
Cr-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528}
[modify] https://crrev.com/0aa6d012be78159e54fc454ea00737a44b124a4a/android_webview/browser/aw_safe_browsing_whitelist_manager.cc
[modify] https://crrev.com/0aa6d012be78159e54fc454ea00737a44b124a4a/android_webview/browser/aw_safe_browsing_whitelist_manager_unittest.cc

Comment 6 by sgu...@chromium.org, Jul 31 2017

Status: Fixed (was: Assigned)
Project Member

Comment 7 by bugdroid1@chromium.org, Jul 31 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/975e3ad6c66f9a4bd0980362ccc57955bbaf83ef

commit 975e3ad6c66f9a4bd0980362ccc57955bbaf83ef
Author: spqchan <spqchan@chromium.org>
Date: Mon Jul 31 17:22:11 2017

Add  missing availability annotations for macOS 10.12.

https://chromium-review.googlesource.com/c/572149/ was
landed without any availability annotations. This causes
"-Wunguarded-availability" warnings to be triggered when
compiling against the 10.12 SDK.

Bug:  747693 
Change-Id: I08a6bc1937bebf17f1d05f6185cb6233472b36a3
Reviewed-on: https://chromium-review.googlesource.com/583850
Reviewed-by: Erik Chen <erikchen@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Sarah Chan <spqchan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#489679}
(cherry picked from commit 1a10b0cc3a3c52b16ea8734aab307ebee28c3496)

Whitelisting logic should not check non-ws non-http urls

Safebrowsing only works on certain schemes. Further the whitelisting
logic has certain assumptions about the URLs that are checked for
whitelisting. Early out if the url does not have the right scheme.

BUG= 750280 
TBR=sgurun@google.com

(cherry picked from commit 6146a7c8cdf03d79a6a0df09c7341a65602c55cf)

Reviewed-on: https://chromium-review.googlesource.com/592227
Commit-Queue: Selim Gurun <sgurun@chromium.org>
Reviewed-by: Eric Roman <eroman@chromium.org>
Cr-Original-Original-Commit-Position: refs/heads/master@{#490589}
Reviewed-on: https://chromium-review.googlesource.com/594327
Reviewed-by: Selim Gurun <sgurun@chromium.org>
Cr-Original-Commit-Position: refs/branch-heads/3163@{#157}
Cr-Original-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528}
Change-Id: I6507e8e0197ed3c1a3dd0eb10e7379cf5ceb459b
Reviewed-on: https://chromium-review.googlesource.com/594469
Reviewed-by: Sarah Chan <spqchan@chromium.org>
Cr-Commit-Position: refs/branch-heads/3163@{#159}
Cr-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528}
[modify] https://crrev.com/975e3ad6c66f9a4bd0980362ccc57955bbaf83ef/chrome/browser/ui/cocoa/autofill/autofill_popup_view_cocoa.h
[modify] https://crrev.com/975e3ad6c66f9a4bd0980362ccc57955bbaf83ef/chrome/browser/ui/cocoa/autofill/autofill_popup_view_cocoa.mm
[modify] https://crrev.com/975e3ad6c66f9a4bd0980362ccc57955bbaf83ef/chrome/browser/ui/cocoa/autofill/autofill_popup_view_cocoa_unittest.mm
[modify] https://crrev.com/975e3ad6c66f9a4bd0980362ccc57955bbaf83ef/chrome/browser/ui/cocoa/tab_contents/tab_contents_controller.mm
[modify] https://crrev.com/975e3ad6c66f9a4bd0980362ccc57955bbaf83ef/chrome/browser/ui/cocoa/web_textfield_touch_bar_controller.h

Comment 8 by sgu...@chromium.org, Jul 31 2017

don't know why the last commit message landed in this bug. unrelated.
Sorry, I think I messed up when I was cherry picking my CL

Sign in to add a comment