New issue
Advanced search Search tips

Issue 799906 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 3
Type: Bug



Sign in to add a comment

Failing DCHECK for invalid favicon url from content setting

Project Member Reported by dullweber@chromium.org, Jan 8 2018

Issue description

Chrome Version: Chromium master
OS: Linux

What steps will reproduce the problem?
(1) Create a content setting like "http://[*.]example.com"

What is the expected result?
Nothing unexpected happens.

What happens instead?
Chromium hits a DCHECK when fetching a favicon:

[98081:98081:0108/125240.034710:FATAL:gurl.cc(168)] Check failed: false. Trying to get the spec of an invalid URL: http://[%2A.]google.de/
#0 0x7fb21497194c base::debug::StackTrace::StackTrace()
#1 0x7fb21499bd1c logging::LogMessage::~LogMessage()
#2 0x7fb213a63e70 GURL::spec()
#3 0x5561b1985d1c FaviconSource::HandleMissingResource()
#4 0x5561b1985c7c FaviconSource::OnFaviconDataAvailable()
#5 0x5561b0fa0968 favicon::FaviconServiceImpl::RunFaviconRawBitmapCallbackWithBitmapResults()
#6 0x5561b0f374fa history::(anonymous namespace)::RunWithFaviconResults()

 
Owner: dullweber@chromium.org
I created a CL to fix the DCHECK: https://crrev.com/c/853864
Project Member

Comment 2 by bugdroid1@chromium.org, Jan 10 2018

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

commit e63a67fd7ab4dbdfb2b2238992bb8862e46bfba1
Author: Christian Dullweber <dullweber@chromium.org>
Date: Wed Jan 10 10:36:56 2018

Prevent failing DCHECK for invalid favicon url

When adding a content setting like "http://[*.]example.com", we try to
fetch a synced favicon for it, which fails with a DCHECK because
GURL::spec complains that this is an invalid url. This CL sanitizes pattern
with a [*.] wildcard and tries to load the favicon for http://example.com
instead.

Before:
https://screenshot.googleplex.com/8hQ6xE5gfay.png
After:
https://screenshot.googleplex.com/v2KiP15LWsi.png

Bug:  799906 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: Idb77abf5e4747bd4415b9e0a4ae156b9a3db93c7
Reviewed-on: https://chromium-review.googlesource.com/853864
Reviewed-by: Dave Schuyler <dschuyler@chromium.org>
Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Commit-Queue: Christian Dullweber <dullweber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#528276}
[modify] https://crrev.com/e63a67fd7ab4dbdfb2b2238992bb8862e46bfba1/chrome/browser/resources/settings/site_settings/site_settings_behavior.js

Status: Fixed (was: Untriaged)

Sign in to add a comment