Closed
Bug 275529
Opened 20 years ago
Closed 19 years ago
Extension Manager does not reject invalid GUIDs
Categories
(Toolkit :: Add-ons Manager, defect)
Toolkit
Add-ons Manager
Tracking
()
RESOLVED
FIXED
mozilla1.8final
People
(Reporter: jens.b, Assigned: jens.b)
References
Details
Attachments
(2 obsolete files)
Currently, the Extension Manager does enforce valid version numbering for packages, but not valid GUIDs. There are packages out there (also listed on update.mozilla.org) that use stuff like {noia_yop-76b1-4bcf-aff9-90e1a5_win32}, which clearly violates the specification. This should be fixed soon, before more authors rely on EM's misbehaviour. Steps to reproduce: 1. Install the "Open link in..." extension from https://addons.update.mozilla.org/extensions/moreinfo.php?application=firefox&id=379&vid=1211 Actual results: Firefox installs the package without complaining. Expected results: Firefox should reject the package, because it uses the invalid GUID "{openlink-0fa3-4886-8adf-24a3e6301ec5}". Patch coming up.
Assignee | ||
Comment 1•20 years ago
|
||
Proposing patch. Should the malformed GUID should be shown to the user? With this patch, the text wrapping in the error message looks a bit ugly.
Comment 3•20 years ago
|
||
IMO, the specification intends <em:id> to be globally unique value, a GUID simply furthers that cause because of it's low chances of two authors using the same one, (w/o copy/paste errors anyway). I don't see a valid reason why the unique identifier must be a valid GUID, when Firefox doesn't depend on it being valid, it depends on it being unique. Its used for the directory name under extensions/ in user's profiles and in communications with the extension/theme update service. The root issue here is obviously that some authors find the GUID difficult to remember and find that adding human-readble aids to their items ID assists them in development. If that causes no problems, then I don't see why the ID can't be flexable, as it is now. Though more importantly, the extensions/themes that users have installed.. ignoring authors and developers for a moment, are going to have their upgrade path broken as far as E/T Update is concerned if the new items suddenly are forced into having new GUIDs. With regard to UMO, the DB changes will cause the old items to be not-found and therefore the users will still have versions that will be defunct with Firefox 1.1, if they upgrade. and smooth migration will fail to find an updated version, even if it exists. If authors update their custom update.rdfs, the same should occur there as well. Is this change worth breaking the update paths for the installed-user base for those items? And... Before you say the authors were at fault for using invalid GUIDs and so-be-it. Consider the user who has no clue what a GUID is at all, who expects things to work as-advertised. Without breakage caused by internal struggles over the meaning of a specification within the organization that created the browser they use.
Assignee | ||
Comment 4•20 years ago
|
||
I think some numbers would help here - whether we're talking about 10 or 100 affected packages could make a huge difference. Would you mind doing a statistic about valid/invalid GUIDs in update.mozilla.org's database? (The appropriate regex is at bug 271270 comment 4, but I'd be happy to help, provided you put up a DB export containing all package IDs somewhere).
Valid to invalid is approximates 20:1, last time I poked this issue.
Comment 6•20 years ago
|
||
IMHO, notify and continue (or confirm) install is the way to go, instead of screening out all malformed ids. (In reply to comment #1) > Created an attachment (id=169284) [edit] As long as ids are case sensitive, it's not "clear-cut specification", anyway. We can install {DF8E5247-8E0A-4de6-B393-0735A39DFD80} and {df8e5247-8e0a-4de6-b393-0735a39dfd80} at the same time.
Comment 7•20 years ago
|
||
GUIDs/UUIDs are, by definition, not case-sensitive. Whether the EM handles them in a case-insensitive way is another matter, though... I'd love to try installing two extensions with IDs differing only by case on a computer with a case-insensitive file system. ;)
Assignee | ||
Comment 8•20 years ago
|
||
Assignee | ||
Updated•20 years ago
|
Attachment #169284 -
Attachment is obsolete: true
Attachment #169766 -
Flags: review?(bsmedberg)
Assignee | ||
Updated•20 years ago
|
Attachment #169284 -
Flags: review?(bsmedberg)
Assignee | ||
Updated•20 years ago
|
Target Milestone: --- → Firefox1.1
Comment 9•19 years ago
|
||
Could this bug be extended to also reject extensions which GUID is the same as GUID which belong to the one of mozilla applications (extra: nvu, netscape, 'new' mozilla suite...). The warning could be sth like: "%S could not install "%S" because its ID ("%S") is used by "%S". Please contact the author about this problem." I saw at least one case when author used FF GUID for his extension ID (either by a mistake or lack of knowledge) and imagine what would happen if user would install 2 such 'buggy' extensions... Or I should file another bug ?
Updated•19 years ago
|
Flags: blocking-aviary1.1?
Comment 10•19 years ago
|
||
A GUID database for all extensions and themes would be an amazing. I has happened that someone used the same GUID as another extension and a searchable database would be a good idea. I could set up this on my server with a simple form to add the GUID, the name of the extension/theme, the type (theme for Firefox/Thunderbird...), The author and a link to a place where I could download it. Then there could be a regex search field so you could check if the GUID is in use.
Comment 11•19 years ago
|
||
It's a GUID... if you're using a guid-generating tool, the chances of a conflict are in the 1/many billion range. But I'm leaning towards allowing these IDs to be pretty strings (and even suggesting that authors use contractid-like strings): @myextension.mozdev.org/1 I still need to work out some details with Ben about directory paths in the EM.
Assignee | ||
Comment 12•19 years ago
|
||
(In reply to comment #11) > I'm leaning towards allowing these IDs to be pretty strings Benjamin: I'm looking forward to having a final answer on the GUIDs-or-not question soon, as it is a much discussed topic. Oh, and if you and Ben go for pretty strings, I'd propose to have them not fully free-form, but with some rules (e.g. startswith/contains @, "domain" with at least one dot, valid tld, at least one slash etc.) This would prevent people from just using something simple as "bettertabs". (In reply to comment #10) > A GUID database for all extensions and themes (...) Marius, this bug is about rejecting *invalid* (meaning malformed) GUIDs, not *conflicting* GUIDs. As Benjamin wrote, proper usage of GUID-generating tools results in a near-zero possibility of collisions. (In reply to comment #9) > Could this bug be extended to also reject extensions which GUID is the same as > GUID which belong to the one of mozilla applications Przemyslaw, this should be a separate bug. However, I guess it's very likely to be WONTFIXed, as it either requires a hard-coded list of the GUIDs of other products (which will never include the whole set), or an on-line service, which is overkill. (OTOH, you can just try filing it, I'm not the one who decides)
Comment 13•19 years ago
|
||
Please keep in mind that there already is a database of all of these GUIDs; they're in UMO. If you change the size of the GUID to allow it to be longer, it will impact UMO. The nice thing about GUIDs is that they follow a specific format, are of constant length, and are easily generated. If the concern is the name of the folder on disk, then maybe that shouldn't be based on the GUID. Perhaps some name space can be used and all extensions from one author could go into that same namespaced folder.
Comment 14•19 years ago
|
||
Alan, the concern is not the filename on disk, it's the fact that GUIDs are not readable. If UMO has fixed-length fields, I will need to talk to them about making it a varchar or somesuch instead. UMO currently holds invalid GUIDs, so I'm pretty sure that they aren't doing rigorous guid checking.
Comment 15•19 years ago
|
||
You're referring to bug 271270 and bug 273550
Assignee | ||
Comment 16•19 years ago
|
||
(In reply to comment #14) > If UMO has fixed-length fields, I will need to talk to them about > making it a varchar or somesuch instead. UMO currently holds invalid GUIDs, so > I'm pretty sure that they aren't doing rigorous guid checking. The GUID field is a varchar(50), so no problems here. Note that the request to change UMO so it enforces valid GUIDs - bug 271270 - is blocked by this bug.
Comment 17•19 years ago
|
||
This bug was fixed by a checkin for bug 286034 . Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8b2) Gecko/20050426 Firefox/1.0+
Comment 18•19 years ago
|
||
(In reply to comment #17) > This bug was fixed by a checkin for bug 286034 . Can you point to the section in LXR that does this?
Comment 19•19 years ago
|
||
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/toolkit/mozapps/extensions/src/nsExtensionManager.js.in&rev=1.93&root=/cvsroot&mark=163-166,927,960,2924#160 We're still discussing whether or what we want to enforce.
Comment 20•19 years ago
|
||
Would be nice: {forecast-7E9B-4d49-8831-A227C80A7AD3} {prefbutt-E113-4f5f-B052-EDB33FE943FF} and everyone would be happy (there's not much choice between A and F to make the extension more recognizable)!
Comment 21•19 years ago
|
||
The two "examples" in comment 20 are very bad and should not be allowed: if you have something GUID-like, it should be a GUID, generated properly by a guidgen tool, not a hand-edited text string that is likely not to be unique. My current thinking (after 1.1a) is to allow two forms of "id": {GUID} (a real GUID) @myextension.mozdev.org;1 (something vaguely contractid-like) The second form would need to avoid characters that shouldn't be in filesystems, so it will allow only the following characters: a-z 0-9 - . @ , ; _ So I think a decent regex would be: m/^(\{[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}\}|\@[a-z0-9\.\-\@;_]+)$/i Does this make sense?
Comment 22•19 years ago
|
||
Another idea for an ID would be this fix form: makelink@rory.parle tabx@stephen.clavering smoothwheel@avi.halachmi externalapp@torisugari I think this kind of human-readable ID is more user friendly than digits. Just an idea. Please ignore this comment if not usable.
Comment 23•19 years ago
|
||
The patch I posted in bug 293461 "fixes" this so that email-like IDs can be used in addition to {GUID}s
Updated•19 years ago
|
Attachment #169766 -
Flags: review?(benjamin)
Comment 24•19 years ago
|
||
(In reply to comment #23) > The patch I posted in bug 293461 "fixes" this so that email-like IDs can be used > in addition to {GUID}s could you please give an example of an email-like IDs
Comment 25•19 years ago
|
||
duplicatetab@twanno.mozdev.org works as ID
Assignee | ||
Updated•19 years ago
|
Attachment #169766 -
Attachment is obsolete: true
Assignee | ||
Comment 26•19 years ago
|
||
Fixed by bsmedberg in bug 293461, resolving.
Comment 27•19 years ago
|
||
Sorry, I wasn't aware of the new possibility (see comment #23 ) for a long time. There are problems with extensions with human readable names: All renamed extensions show up, also the chrome shows up, but most of them don't (fully) work. I tried renaming Search Button 0.4.8: http://www.extensionsmirror.nl/index.php?showtopic=95 Make Link 2.0.1: http://www.extensionsmirror.nl/index.php?showtopic=64 and Paste Happy 0.6.1: http://www.extensionsmirror.nl/index.php?showtopic=531 Only Paste Happy seems to work. Make Link works partially (only "Configure...") and the Search Button does show up, but doesn't work.
Comment 28•19 years ago
|
||
I see the same when installing an xpi via my desktop with a changed id: it installs but doesn't work entirely. I appreciate this new feature very much. Thanks for that. Makes it much easier for me to edit extensions.
Comment 29•19 years ago
|
||
Only Makelink and Searchbutton seem to have problems, the rest was due to a spoiled registration.
Updated•19 years ago
|
Flags: blocking-aviary1.1?
Updated•16 years ago
|
Product: Firefox → Toolkit
You need to log in
before you can comment on or make changes to this bug.
Description
•