New issue
Advanced search Search tips

Issue 894275 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 12
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 1
Type: Bug

Blocking:
issue 696822



Sign in to add a comment

DNR: Browser crashes when loading a DNR ruleset with multiple rules which can't be parsed.

Project Member Reported by karandeepb@chromium.org, Oct 10

Issue description

Try to load an extension with the following ruleset:

[
        {
          "id" : 1,
          "action": { "type" : "invalid_action" },
          "condition" : {"urlFilter" : "google.com"}
        },
        {
          "id" : 2,
          "action" : { "type" : "invalid_action_2" },
          "condition" : { "urlFilter" : "google.com" }
        },
]

The browser will crash.
      
 
Blocking: 696822
Status: Started (was: Assigned)
Project Member

Comment 3 by bugdroid1@chromium.org, Oct 12

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

commit 432a8ffc0cbaa1999069657e2ba360ea910ba961
Author: Karan Bhatia <karandeepb@chromium.org>
Date: Fri Oct 12 21:09:56 2018

DNR: Fix handling of install warnings for rule resources file.

This CL make the following changes:
- Fixes a potential null-dereference crash when multiple rules couldn't be
  parsed.
- Instead of showing only a single install warning for multiple unparsed rules,
  show individual warnings. Limit the count to 5 warnings.
- The rule count exceeded warning doesn't override the rule unparsed warning
  now.

BUG= 894275 

Change-Id: Iac8db1cf927a9302ec9295501f84616efafeca4e
Reviewed-on: https://chromium-review.googlesource.com/c/1274838
Commit-Queue: Karan Bhatia <karandeepb@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#599352}
[modify] https://crrev.com/432a8ffc0cbaa1999069657e2ba360ea910ba961/chrome/browser/extensions/api/declarative_net_request/rule_indexing_unittest.cc
[modify] https://crrev.com/432a8ffc0cbaa1999069657e2ba360ea910ba961/extensions/browser/api/declarative_net_request/constants.cc
[modify] https://crrev.com/432a8ffc0cbaa1999069657e2ba360ea910ba961/extensions/browser/api/declarative_net_request/constants.h
[modify] https://crrev.com/432a8ffc0cbaa1999069657e2ba360ea910ba961/extensions/browser/api/declarative_net_request/utils.cc

Status: Fixed (was: Started)
Labels: Merge-Request-71 OS-Chrome OS-Linux OS-Mac OS-Windows
Requesting merge to M71. I plan to enable the Declarative Net Request API on M71. See crbug.com/886497 for the approval.
Project Member

Comment 6 by sheriffbot@chromium.org, Nov 20

Labels: -Merge-Request-71 Hotlist-Merge-Review Merge-Review-71
This bug requires manual review: We are only 13 days from stable.
Please contact the milestone owner if you have questions.
Owners: benmason@(Android), kariahda@(iOS), kbleicher@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Why this merge request so late? M71 is going to stable on Dec 4th, we only have one more beta release left before stable promotion.

crbug.com/886497  - It was approved for Beta on Oct 25th. 


Labels: -Hotlist-Merge-Review -Merge-Review-71
Sorry. I just saw the release schedule. I was going to withdraw the merge request, this shouldn't be required. We can wait for the M72 beta. (It was approved on Nov 8 actually).
Cc: gov...@chromium.org
cc:govind@ in case you didn't receive the above message.
Thank you  karandeepb@.

Sign in to add a comment