DNR: Tracker for issues related to the flatbuffer ruleset file. |
||
Issue descriptionThe flatbuffer ruleset file for DNR would be persisted in the _metadata folder in the extension directory. This bug tracks the associated gotchas: 1) The ruleset file should not be tracked by content verification. 2) Reloading an unpacked extension should not cause spurious warnings due to the presence of _metadata directory. 3) If a package extension being installed or an unpacked extension being loaded, already has a file with the same name as the flatbuffer ruleset file, that file should not affect DNR behavior. DNR should not be tricked into using that file as the ruleset. 4) If an unpacked extension using DNR is zipped and uploaded to CWS, it will still contain the _metadata directory. CWS should just delete the _metadata folder, since these rulesets can be pretty big.
,
Sep 11 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/46178663e6ce7174c7e22957fb3a8a167bcdab63 commit 46178663e6ce7174c7e22957fb3a8a167bcdab63 Author: Karan Bhatia <karandeepb@chromium.org> Date: Mon Sep 11 21:50:45 2017 Extensions: Delete generated _metadata folder before unpacked extension load. This CL changes how _metadata folder is handled during load for unpacked extensions. This is done to prevent spurious warnings on extension reload due to the presence of a generated _metadata folder and to ensure that we are somehow not tricked into using a flatbuffer file provided by an extension for the Declarative Net Request API. Currently, presence of the _metadata folder causes a warning while loading extensions (r492079). This change will remove _metadata folder on unpacked extension load when it only contains known Extension system files that are safe to remove. Loading unpacked extensions with non-reserved filenames in the _metadata folder should now yield an error. Loading unpacked extensions with only reserved filenames (eg computed_hashes.json) should work, with the _metadata folder getting deleted on extension load. Loading a packed extension with a _metadata folder should still yield a warning. BUG= 402278 , 696822, 760850 Change-Id: Ie7daf6450757702353d1d4fc8b96181eba2f669d Reviewed-on: https://chromium-review.googlesource.com/656567 Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org> Commit-Queue: Karan Bhatia <karandeepb@chromium.org> Cr-Commit-Position: refs/heads/master@{#501052} [modify] https://crrev.com/46178663e6ce7174c7e22957fb3a8a167bcdab63/chrome/browser/extensions/extension_service_unittest.cc [modify] https://crrev.com/46178663e6ce7174c7e22957fb3a8a167bcdab63/chrome/browser/extensions/unpacked_installer.cc [modify] https://crrev.com/46178663e6ce7174c7e22957fb3a8a167bcdab63/chrome/browser/extensions/unpacked_installer.h [delete] https://crrev.com/0e2fc3a0b945320fb73188181858661f545af9f3/chrome/test/data/extensions/underscore_metadata_folders/_badfolder/empty_doc [delete] https://crrev.com/0e2fc3a0b945320fb73188181858661f545af9f3/chrome/test/data/extensions/underscore_metadata_folders/_metadata/empty_doc [delete] https://crrev.com/0e2fc3a0b945320fb73188181858661f545af9f3/chrome/test/data/extensions/underscore_metadata_folders/manifest.json [delete] https://crrev.com/0e2fc3a0b945320fb73188181858661f545af9f3/chrome/test/data/extensions/underscore_name/_badfolder/empty_doc [delete] https://crrev.com/0e2fc3a0b945320fb73188181858661f545af9f3/chrome/test/data/extensions/underscore_name/manifest.json [modify] https://crrev.com/46178663e6ce7174c7e22957fb3a8a167bcdab63/extensions/browser/api/declarative_net_request/utils.cc [modify] https://crrev.com/46178663e6ce7174c7e22957fb3a8a167bcdab63/extensions/browser/api/declarative_net_request/utils.h [modify] https://crrev.com/46178663e6ce7174c7e22957fb3a8a167bcdab63/extensions/common/constants.cc [modify] https://crrev.com/46178663e6ce7174c7e22957fb3a8a167bcdab63/extensions/common/constants.h [modify] https://crrev.com/46178663e6ce7174c7e22957fb3a8a167bcdab63/extensions/common/file_util.cc [modify] https://crrev.com/46178663e6ce7174c7e22957fb3a8a167bcdab63/extensions/common/file_util.h [modify] https://crrev.com/46178663e6ce7174c7e22957fb3a8a167bcdab63/extensions/common/file_util_unittest.cc
,
Sep 14 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/63c02145ef64b5cb2de3b2629e9cfd213ec33927 commit 63c02145ef64b5cb2de3b2629e9cfd213ec33927 Author: Alexander Alekseev <alemate@chromium.org> Date: Thu Sep 14 19:19:19 2017 Revert "Extensions: Delete generated _metadata folder before unpacked extension load." This reverts commit 46178663e6ce7174c7e22957fb3a8a167bcdab63. Reason for revert: This breaks Chrome OS PFQ. See https://crbug.com/764787 for details. Original change's description: > Extensions: Delete generated _metadata folder before unpacked extension load. > > This CL changes how _metadata folder is handled during load for unpacked > extensions. This is done to prevent spurious warnings on extension reload due to > the presence of a generated _metadata folder and to ensure that we are somehow > not tricked into using a flatbuffer file provided by an extension for the > Declarative Net Request API. Currently, presence of the _metadata folder causes > a warning while loading extensions (r492079). This change will remove _metadata > folder on unpacked extension load when it only contains known Extension system > files that are safe to remove. > > Loading unpacked extensions with non-reserved filenames in the _metadata folder > should now yield an error. Loading unpacked extensions with only reserved > filenames (eg computed_hashes.json) should work, with the _metadata folder > getting deleted on extension load. Loading a packed extension with a _metadata > folder should still yield a warning. > > BUG= 402278 , 696822, 760850 > > Change-Id: Ie7daf6450757702353d1d4fc8b96181eba2f669d > Reviewed-on: https://chromium-review.googlesource.com/656567 > Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org> > Commit-Queue: Karan Bhatia <karandeepb@chromium.org> > Cr-Commit-Position: refs/heads/master@{#501052} TBR=lazyboy@chromium.org,karandeepb@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 764787, 402278 , 696822, 760850 Change-Id: Ie489f38d1c08668957e393f31ccabcd6339f49c5 Reviewed-on: https://chromium-review.googlesource.com/666737 Commit-Queue: Mitsuru Oshima <oshima@chromium.org> Reviewed-by: Alexander Alekseev <alemate@chromium.org> Cr-Commit-Position: refs/heads/master@{#502012} [modify] https://crrev.com/63c02145ef64b5cb2de3b2629e9cfd213ec33927/chrome/browser/extensions/extension_service_unittest.cc [modify] https://crrev.com/63c02145ef64b5cb2de3b2629e9cfd213ec33927/chrome/browser/extensions/unpacked_installer.cc [modify] https://crrev.com/63c02145ef64b5cb2de3b2629e9cfd213ec33927/chrome/browser/extensions/unpacked_installer.h [add] https://crrev.com/63c02145ef64b5cb2de3b2629e9cfd213ec33927/chrome/test/data/extensions/underscore_metadata_folders/_badfolder/empty_doc [add] https://crrev.com/63c02145ef64b5cb2de3b2629e9cfd213ec33927/chrome/test/data/extensions/underscore_metadata_folders/_metadata/empty_doc [add] https://crrev.com/63c02145ef64b5cb2de3b2629e9cfd213ec33927/chrome/test/data/extensions/underscore_metadata_folders/manifest.json [add] https://crrev.com/63c02145ef64b5cb2de3b2629e9cfd213ec33927/chrome/test/data/extensions/underscore_name/_badfolder/empty_doc [add] https://crrev.com/63c02145ef64b5cb2de3b2629e9cfd213ec33927/chrome/test/data/extensions/underscore_name/manifest.json [modify] https://crrev.com/63c02145ef64b5cb2de3b2629e9cfd213ec33927/extensions/browser/api/declarative_net_request/utils.cc [modify] https://crrev.com/63c02145ef64b5cb2de3b2629e9cfd213ec33927/extensions/browser/api/declarative_net_request/utils.h [modify] https://crrev.com/63c02145ef64b5cb2de3b2629e9cfd213ec33927/extensions/common/constants.cc [modify] https://crrev.com/63c02145ef64b5cb2de3b2629e9cfd213ec33927/extensions/common/constants.h [modify] https://crrev.com/63c02145ef64b5cb2de3b2629e9cfd213ec33927/extensions/common/file_util.cc [modify] https://crrev.com/63c02145ef64b5cb2de3b2629e9cfd213ec33927/extensions/common/file_util.h [modify] https://crrev.com/63c02145ef64b5cb2de3b2629e9cfd213ec33927/extensions/common/file_util_unittest.cc
,
Sep 16 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/fe12281a3f698668a660ef62ee00ae22eca25221 commit fe12281a3f698668a660ef62ee00ae22eca25221 Author: Karan Bhatia <karandeepb@chromium.org> Date: Sat Sep 16 02:38:18 2017 Reland: Extensions: Delete generated _metadata folder before unpacked extension load. r501052 started giving a hard error while loading extensions from the command line, if they contained an illegal file name. This broke Chrome OS autotests and the CL had to be reverted. This CL relands it. Command line extensions with illegal file names now no longer yield a hard error, as was the case before r501052. Original Description: > This CL changes how _metadata folder is handled during load for unpacked > extensions. This is done to prevent spurious warnings on extension reload due to > the presence of a generated _metadata folder and to ensure that we are somehow > not tricked into using a flatbuffer file provided by an extension for the > Declarative Net Request API. Currently, presence of the _metadata folder causes > a warning while loading extensions (r492079). This change will remove _metadata > folder on unpacked extension load when it only contains known Extension system > files that are safe to remove. > > Loading unpacked extensions with non-reserved filenames in the _metadata folder > should now yield an error. Loading unpacked extensions with only reserved > filenames (eg computed_hashes.json) should work, with the _metadata folder > getting deleted on extension load. Loading a packed extension with a _metadata > folder should still yield a warning. BUG= 402278 , 696822, 760850 Change-Id: I413783d4f2c325c0fc834ba27bb1e16c3ce4ff21 Reviewed-on: https://chromium-review.googlesource.com/668163 Commit-Queue: Karan Bhatia <karandeepb@chromium.org> Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org> Cr-Commit-Position: refs/heads/master@{#502478} [modify] https://crrev.com/fe12281a3f698668a660ef62ee00ae22eca25221/chrome/browser/extensions/extension_service_unittest.cc [modify] https://crrev.com/fe12281a3f698668a660ef62ee00ae22eca25221/chrome/browser/extensions/unpacked_installer.cc [modify] https://crrev.com/fe12281a3f698668a660ef62ee00ae22eca25221/chrome/browser/extensions/unpacked_installer.h [delete] https://crrev.com/b1e1e775c874eb2a05e8695ecd8e6cc1c36ce5b8/chrome/test/data/extensions/underscore_metadata_folders/_badfolder/empty_doc [delete] https://crrev.com/b1e1e775c874eb2a05e8695ecd8e6cc1c36ce5b8/chrome/test/data/extensions/underscore_metadata_folders/_metadata/empty_doc [delete] https://crrev.com/b1e1e775c874eb2a05e8695ecd8e6cc1c36ce5b8/chrome/test/data/extensions/underscore_metadata_folders/manifest.json [delete] https://crrev.com/b1e1e775c874eb2a05e8695ecd8e6cc1c36ce5b8/chrome/test/data/extensions/underscore_name/_badfolder/empty_doc [delete] https://crrev.com/b1e1e775c874eb2a05e8695ecd8e6cc1c36ce5b8/chrome/test/data/extensions/underscore_name/manifest.json [modify] https://crrev.com/fe12281a3f698668a660ef62ee00ae22eca25221/extensions/browser/api/declarative_net_request/utils.cc [modify] https://crrev.com/fe12281a3f698668a660ef62ee00ae22eca25221/extensions/browser/api/declarative_net_request/utils.h [modify] https://crrev.com/fe12281a3f698668a660ef62ee00ae22eca25221/extensions/common/constants.cc [modify] https://crrev.com/fe12281a3f698668a660ef62ee00ae22eca25221/extensions/common/constants.h [modify] https://crrev.com/fe12281a3f698668a660ef62ee00ae22eca25221/extensions/common/file_util.cc [modify] https://crrev.com/fe12281a3f698668a660ef62ee00ae22eca25221/extensions/common/file_util.h [modify] https://crrev.com/fe12281a3f698668a660ef62ee00ae22eca25221/extensions/common/file_util_unittest.cc
,
Sep 5
Think this can be closed. >> 1) The ruleset file should not be tracked by content verification. This was implemented. >> 2) Reloading an unpacked extension should not cause spurious warnings due to the presence of _metadata directory. This was also implemented. >> 3) If a package extension being installed or an unpacked extension being loaded, already has a file with the same name as the flatbuffer ruleset file, that file should not affect DNR behavior. DNR should not be tricked into using that file as the ruleset. Checksum verification should ensure this. >> 4) If an unpacked extension using DNR is zipped and uploaded to CWS, it will still contain the _metadata directory. CWS should just delete the _metadata folder, since these rulesets can be pretty big. Confirmed that CWS will disallow uploading packages with a _metadata directory. |
||
►
Sign in to add a comment |
||
Comment 1 by lazyboy@chromium.org
, Aug 31 2017