509946 - Add-ons with invalid types should be rejected when uploaded

Status: RESOLVED FIXED

(addons.mozilla.org Graveyard :: Developer Pages, defect, P3)

Description

[Dave Townsend [:mossop]]

Currently you can apparently upload add-ons with invalid types in their install.rdf (add-on 13648 has type 3 f.e.). AMO should only accept add-ons with a known type.

Comment 1

[Fred Wenzel [:wenzel]]

"New in Firefox 1.5 This property was added for Firefox 1.5, and is only required for addon types other than Extensions and Themes."

In other words: It should either be undefined or valid.

Comment 2

[Dave Townsend [:mossop]]

Sorry yes. A missing type should still be considered as type 2 or 4 as currently.

Comment 3

[Jorge Villalobos [:jorgev] (he/him)]

I've seen cases where Themes are uploaded without a defined type and they are recognized as Extensions by AMO. This causes some breakage, like incorrect categories showing up in the add-on properties.

I'd prefer to make the type attribute mandatory for AMO, even if Firefox doesn't do this. And, of course, reject unknown types.

Comment 4

[Wil Clouser [:clouserw]]

What are the rules to validate here (ie. what are the types)?  Basta is already detecting add-on types, so this is easy to add.

Comment 5

[Dave Townsend [:mossop]]

(In reply to comment #4)
> What are the rules to validate here (ie. what are the types)?  Basta is already
> detecting add-on types, so this is easy to add.

The types are listed here: https://developer.mozilla.org/en/Install_Manifests#type

If no type is specified but em:internalName exists then the type is considered to be 4 (theme). If no type or internalName exists then the type is 2 (extension)

Comment 6

[Matt Basta [:basta]]

The new validator does a number of things for the add-on's type:

- Allows a type to be manually set. If the detected type does not match this, then the add-on is marked as rejected.
- Determines the type based on the developer's indication (i.e.: <em:type>). If this does not match the detected type, the add-on is rejected.
- Detects the type based on filename, file content, package structure, and other factors.

The detected type is what is used by the validator during any processing of the add-on beyond type detection.

Comment 7

[Matt Basta [:basta]]

Just looked it up: this is already implemented in the new validator and is present in the validator's zamboni implementation.

Comment 8

[krupa raj[:krupa]]

I was able to upload an add-on with <em:type>21</em:type> . I did not see any error messages.

reopening.

Comment 9

[Matt Basta [:basta]]

I'm assuming you tested on the new validator. Could you email me a copy of the add-on that you uploaded? There's definitely code in there that *should* be throwing errors for invalid <em:type> values (and there should be unit tests for), but if it's failing, I'll need to see what's causing it.

[e-mail]

Thanks!

Comment 10

[krupa raj[:krupa]]

I did not know remora still pointed to the old validator. My mistake.