Plugin blocks don't allow simultaneous plugin version and application version filters

VERIFIED FIXED in Firefox 17

Status

()

Toolkit
Blocklisting
--
critical
VERIFIED FIXED
5 years ago
a year ago

People

(Reporter: jorgev, Assigned: wraithan)

Tracking

unspecified
2012-10-25
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox17+ fixed)

Details

(Reporter)

Description

5 years ago
If we want to block a plugin and restrict if by plugin version, we use the Min and Max fields, like in:
https://addons.mozilla.org/admin/models/blocklist/blocklistdetail/138/

If we want to restrict by application version, we use the GUID, Min and Max fields, like in:
https://addons.mozilla.org/admin/models/blocklist/blocklistdetail/27/

Since they use the same fields, they can't be used simultaneously.

Extension blocks work differently. You add the blocklist detail, and then you create a separate blocklist app entry (https://addons.mozilla.org/admin/models/blocklist/blocklistapp/) that handles the application version range. You can link blocklist app entries to extension blocks, but not to plugin blocks.

There are a couple of possible solutions here. One is to expand the form in the plugin section, so that it has separate Min / Max entries for application and plugin version. The other one would be to allow blocklist app entries to be associated with plugin blocks.

This was brought up in bug 793273, and it limited our options for keeping CTP blocks enabled in Firefox 16.
(Reporter)

Comment 1

5 years ago
This is limiting our possibilities when planning plugin blocks, so we need it fixed soon.

Wil, Wraithan, can you look into this?
Severity: major → critical
Target Milestone: --- → 2012-10-11

Updated

5 years ago
tracking-firefox17: --- → +
(Reporter)

Comment 2

5 years ago
(In reply to Jorge Villalobos [:jorgev] from comment #0)
> The other one would be to allow blocklist
> app entries to be associated with plugin blocks.

To answer a question from IRC, I prefer this approach because it is consistent with the way we block extensions.
Wraithan volunteered to check this out.
Assignee: nobody → xwraithanx
(Reporter)

Updated

5 years ago
Blocks: 799546
Have working code, need to write tests and a migration.
Wont be done with enough time for code review/QA, pushing to next week.

WIP code:
https://github.com/wraithan/zamboni/tree/795387
Target Milestone: 2012-10-11 → 2012-10-18

Comment 6

5 years ago
(In reply to Wraithan from comment #5)
> Wont be done with enough time for code review/QA, pushing to next week.
> 
> WIP code:
> https://github.com/wraithan/zamboni/tree/795387

Still on track for this week? This is blocking critical CTP testing on FF17.

Updated

5 years ago
Blocks: 803152
This slipped in favor of getting PINs working for paid apps in the Firefox Marketplace. I should have said something earlier this week and got it handed off. I apologize for that.
Target Milestone: 2012-10-18 → 2012-10-25
https://github.com/mozilla/zamboni/commit/5e99328f6a0210554f0f8d4ec703fbc4d291dc8c
Here's the patch.
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
thanks wraithan
(Reporter)

Comment 10

5 years ago
Reopening because https://addons-dev.allizom.org/en-US/admin/models/blocklist/blocklistapp/ isn't accessible, and I am unable to add new app items.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Reporter)

Comment 11

5 years ago
A migration script wasn't run when pushing. This is fixed now.
Status: REOPENED → RESOLVED
Last Resolved: 5 years ago5 years ago
Resolution: --- → FIXED
status-firefox17: --- → fixed
Will wait until this is in production for marking status-firefox17 fixed.
status-firefox17: fixed → ---
Steps for testing:

You'll be entering BlocklistPlugins and BlocklistApps in the admin here:

http://addons-dev.allizom.org/en-US/admin/models/blocklist/

And verifying it works by looking at the blocklist url:

http://addons-dev.allizom.org/en-US/blocklist/2/<guid>/<version>

guid - This will be a value you come up with.
version - this is a number you come up with.

Try adding a BlocklistPlugin with just a name and details (all that is required)

Make sure it shows up in the blocklist url.

Try adding a BlocklistApp to the BlocklistPlugin, make sure to set the guid to a value you came up with and the version range (min and max) so that the version number will be inside.

Make sure it shows up in your blocklist url.

Change the version numbers in the BlocklistApp that you made to exclude the version number you came up with.
Make sure it isn't in your blocklist url.

Change the version range on the BlocklistApp that you made back to including the version.
Make sure it is in your blocklist url.

Change the guid on the BlocklistApp to something different.
Make sure it isn't in your blocklist url.
(Reporter)

Comment 14

5 years ago
These blocks are supposed to have both filters, but they only show up the application version filter:

<pluginItem  blockID="p163">
  <match name="filename" exp="(NPSWF32\.dll)|(Flash\ Player\.plugin)" />
  <versionRange  severity="0" vulnerabilitystatus="1">
    <targetApplication  id="{ec8030f7-c20a-464f-9b0e-13a3a9e97384}">
      <versionRange  minVersion="17.0" maxVersion="*" />
    </targetApplication>
  </versionRange>
</pluginItem>

<pluginItem  blockID="p165">
  <match name="filename" exp="(NPSWF[0-9_]*\.dll)|(Flash\ Player\.plugin)" />
  <versionRange  severity="0" vulnerabilitystatus="1">
    <targetApplication  id="{ec8030f7-c20a-464f-9b0e-13a3a9e97384}">
      <versionRange  minVersion="17.0" maxVersion="*" />
    </targetApplication>
  </versionRange>
</pluginItem>
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
https://github.com/mozilla/zamboni/commit/d0089feb215791eed993a362d7d83e6bc5b31b9d

forgot to account for plugin.min and plugin.max properly
Status: REOPENED → RESOLVED
Last Resolved: 5 years ago5 years ago
Resolution: --- → FIXED
(Reporter)

Comment 16

5 years ago
Verified in prod.
Status: RESOLVED → VERIFIED
status-firefox17: --- → fixed
Blocks: 843373
No longer blocks: 843373
Product: addons.mozilla.org → Toolkit
You need to log in before you can comment on or make changes to this bug.