639723 - Empty <devices></devices> element output for blocklist entries with no device

Status: RESOLVED FIXED

(Toolkit :: Blocklist Policy Requests, defect)

Description

[Joe Drew (not getting mail)]

For gfx blocklist entries with no devices (i.e., that are supposed to be applied to all devices), from my reading of http://viewvc.svn.mozilla.org/vc/addons/trunk/site/app/webroot/services/blocklist.php?revision=81941&view=markup#l554 , we are going to output <device></device>, which in the current implementation of the graphics blocklist actually translates to "block nothing" instead of "block everything."

It's possible for us to fix this in C++ code, but that'll require a respin of Firefox 4.

Ideally we wouldn't output any gfx blocklist entry that's stored in the database as NULL or an empty string.

Nominated for blocking because it blocks bug 639698, which is also nominated for blocking.

Comment 1

[Les Orchard [:lorchard]]

(In reply to comment #0)

> Ideally we wouldn't output any gfx blocklist entry that's stored in the
> database as NULL or an empty string.

Don't quite understand... just to be clear:

Do you mean ideally that the blocklist should omit the *entire* blocklist entry if any fields are null?

Or, do you mean the blocklist should omit just the empty <devices></devices> element, but include the rest of the entry as a whole?

Comment 2

[Joe Drew (not getting mail)]

The latter.

Comment 3

[Joe Drew (not getting mail)]

Er, and when I said:

> Ideally we wouldn't output any gfx blocklist entry that's stored in the database as NULL or an empty string.

I meant something analagous to that: don't output any tag whose text value is empty or null.

Comment 4

[Les Orchard [:lorchard]]

Cool, that's what I thought, but wanted to double check.

Just committed r84315, which wraps every element in a check whether the corresponding column is empty. It should show up on staging soon.

http://viewvc.svn.mozilla.org/vc/addons/trunk/site/app/webroot/services/blocklist.php?revision=84315&view=markup#l554

Comment 5

[Les Orchard [:lorchard]]

FWIW, I'm not sure there are any gfx blacklist items on staging, and I don't have access to the DB there.

I'm trying these URLs with curl:

https://addons.allizom.org/services/blocklist.php?debug&appGuid=%7Bec8030f7-c20a-464f-9b0e-13a3a9e97384%7D&reqVersion=1.0&appVersion=4.0

https://addons.allizom.org/blocklist/3/%7Bec8030f7-c20a-464f-9b0e-13a3a9e97384%7D/x/x/x/x/x/x/x/x/x/

The first bypasses mod_rewrite to include a debugging flag, while the second is the proper format used by Firefox. I just stuck "x" in for the URL segments that don't figure into the match because I'm lazy, but you can substitute a real service URL if you have one on hand.

Comment 6

[Les Orchard [:lorchard]]

Adding clouserw, since he added testing data for me last time

Comment 7

[Wil Clouser [:clouserw]]

(In reply to comment #6)
> Adding clouserw, since he added testing data for me last time

What do you want?  Give me SQL if you want something run on PAMO.

Comment 8

[Les Orchard [:lorchard]]

(In reply to comment #7)
> (In reply to comment #6)
> > Adding clouserw, since he added testing data for me last time
> 
> What do you want?  Give me SQL if you want something run on PAMO.

From the blocker bug:
INSERT INTO `blgfxdrivers` (NULL, NULL, 'WINNT 5.1', '0x10de', '',
'DIRECT3D_9_LAYERS', 'BLOCKED_DRIVER_VERSION', '7.0.0.0',
'GREATER_THAN_OR_EQUAL', NOW(), NOW())

Comment 9

[Wil Clouser [:clouserw]]

Syntax error without "VALUES" in it, but I fixed it up, 1 row inserted.  I'm on IRC if there is more to do, it'd probably be faster.

Comment 10

[Les Orchard [:lorchard]]

Okay, <devices> element is still showing up, but I'll have a fix in a few.

Comment 11

[Les Orchard [:lorchard]]

r84368, will wait until I see it on staging to close this bug

Comment 12

[Les Orchard [:lorchard]]

$ curl -sD - 'https://addons.allizom.org/blocklist/3/%7Bec8030f7-c20a-464f-9b0e-13a3a9e97384%7D/x/x/x/x/x/x/x/x/x/'

...
<gfxItems>
  <gfxBlacklistEntry>
    <os>WINNT 5.1</os>
    <vendor>0x10de</vendor>
    <feature>DIRECT3D_9_LAYERS</feature>
    <featureStatus>BLOCKED_DRIVER_VERSION</featureStatus>
    <driverVersion>7.0.0.0</driverVersion>
    <driverVersionComparator>GREATER_THAN_OR_EQUAL</driverVersionComparator>
  </gfxBlacklistEntry>
</gfxItems>
...

Marking this fixed, but will need to make it onto AMO production and be verified there. I think Thursday is the next AMO push...?

Comment 13

[Les Orchard [:lorchard]]

Ugh, this Bugzilla Tweaks addon keeps changing that blocking2.0 flag