New issue
Advanced search Search tips

Issue 762754 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Sep 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 1
Type: Bug
Team-Security-UX



Sign in to add a comment

Adding a pattern containing the * wildcard to Site Settings gets encoded as "%2A" on display.

Project Member Reported by patricia...@chromium.org, Sep 7 2017

Issue description

Chrome Version: 63.0.3207.0
OS: Chrome Desktop

What steps will reproduce the problem?
(1) Navigate to chrome://settings/content/flash/
(2) Add a new 'Block' exception for 'http://*'.

What is the expected result?
This should show up as typed, i.e. 'http://*'

What happens instead?
Shows up as 'http://%2A' instead.

Note this works internally, just shows up incorrectly.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Sep 12 2017

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

commit d8476bf05b4af9f48e7bda417b36bce2cac964ab
Author: Patti <patricialor@chromium.org>
Date: Tue Sep 12 01:16:33 2017

MD Settings: Re-separate codepaths for getting display names in Site Settings.

Before r490727, Site Settings / Content Settings always interpreted URLs to be
either an extension (in which the extension name would be retrieved) or a
ContentSettingsPattern. r490727 introduced a third option which was to display
the URL as the origin it belongs to (including stripping any default port
numbers) by checking whether the pattern was a valid origin. However, this
introduced a bug where 'https://*' would be intrepreted as a valid origin and be
displayed as 'https://%2A' in cases where it should have stayed a
ContentSettingsPattern.

Fix this by re-separating the codepaths for getting a pattern display name and
getting an origin display name.

Manual test - Navigate to chrome://settings/content/flash/ and add a new 'Block'
exception for 'http://*'. This should show up as typed and not get converted to
'http://%2A'.

Bug:  656758 ,  762754 
Change-Id: Ifa6c5f94e4485c7e78f1262a357b219e3cb86c19
Reviewed-on: https://chromium-review.googlesource.com/612025
Commit-Queue: Patti <patricialor@chromium.org>
Reviewed-by: Dave Schuyler <dschuyler@chromium.org>
Reviewed-by: Martin Šrámek <msramek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#501136}
[modify] https://crrev.com/d8476bf05b4af9f48e7bda417b36bce2cac964ab/chrome/browser/ui/webui/settings/site_settings_handler_unittest.cc
[modify] https://crrev.com/d8476bf05b4af9f48e7bda417b36bce2cac964ab/chrome/browser/ui/webui/site_settings_helper.cc
[modify] https://crrev.com/d8476bf05b4af9f48e7bda417b36bce2cac964ab/chrome/browser/ui/webui/site_settings_helper_unittest.cc

Cc: lafo...@chromium.org
Labels: -Pri-3 Merge-Request-62 Pri-1
CC laforge@ - apologies, I should have done this earlier.

I'm bumping up the priority to Pri-1 - it should have always been higher, since the use-case described in the bug description is advice given out to users who want to make exceptions for flash deprecation.
Project Member

Comment 3 by sheriffbot@chromium.org, Sep 13 2017

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

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

Comment 4 by bugdroid1@chromium.org, Sep 14 2017

Labels: -merge-approved-62 merge-merged-3202
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/cebdc93527542fe09bcf7f4e496c5905fc0748fa

commit cebdc93527542fe09bcf7f4e496c5905fc0748fa
Author: Patti <patricialor@chromium.org>
Date: Thu Sep 14 14:08:19 2017

MD Settings: Re-separate codepaths for getting display names in Site Settings.

Before r490727, Site Settings / Content Settings always interpreted URLs to be
either an extension (in which the extension name would be retrieved) or a
ContentSettingsPattern. r490727 introduced a third option which was to display
the URL as the origin it belongs to (including stripping any default port
numbers) by checking whether the pattern was a valid origin. However, this
introduced a bug where 'https://*' would be intrepreted as a valid origin and be
displayed as 'https://%2A' in cases where it should have stayed a
ContentSettingsPattern.

Fix this by re-separating the codepaths for getting a pattern display name and
getting an origin display name.

Manual test - Navigate to chrome://settings/content/flash/ and add a new 'Block'
exception for 'http://*'. This should show up as typed and not get converted to
'http://%2A'.

Bug:  656758 ,  762754 
Change-Id: Ifa6c5f94e4485c7e78f1262a357b219e3cb86c19
Reviewed-on: https://chromium-review.googlesource.com/612025
Commit-Queue: Patti <patricialor@chromium.org>
Reviewed-by: Dave Schuyler <dschuyler@chromium.org>
Reviewed-by: Martin Šrámek <msramek@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#501136}(cherry picked from commit d8476bf05b4af9f48e7bda417b36bce2cac964ab)
Reviewed-on: https://chromium-review.googlesource.com/667496
Reviewed-by: Anthony LaForge <laforge@chromium.org>
Cr-Commit-Position: refs/branch-heads/3202@{#219}
Cr-Branched-From: fa6a5d87adff761bc16afc5498c3f5944c1daa68-refs/heads/master@{#499098}
[modify] https://crrev.com/cebdc93527542fe09bcf7f4e496c5905fc0748fa/chrome/browser/ui/webui/settings/site_settings_handler_unittest.cc
[modify] https://crrev.com/cebdc93527542fe09bcf7f4e496c5905fc0748fa/chrome/browser/ui/webui/site_settings_helper.cc
[modify] https://crrev.com/cebdc93527542fe09bcf7f4e496c5905fc0748fa/chrome/browser/ui/webui/site_settings_helper_unittest.cc

Status: Verified (was: Assigned)
Thanks laforge@ for the merge - I was waiting to verify the change works in Canary before merging, but didn't get round to it yet. Have checked now though - verified in 63.0.3215.0.

Sign in to add a comment