Closed Bug 290642 Opened 20 years ago Closed 19 years ago

[Submission] Additem must not error for unrecognized GUIDs

Categories

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

x86
Windows 2000
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: Bugzilla-alanjstrBugs, Assigned: morgamic)

References

()

Details

Attachments

(1 file, 2 obsolete files)

Error! The MinAppVer for Thunderbird of 1.0 in install.rdf is invalid. Error! The MaxAppVer for Thunderbird of 1.0 in install.rdf is invalid. Errors were encountered during install.rdf checking... Aborting...
The problem is that it gets to Nvu, can't find it, and dies. Instead it should realize that zero rows are returned during app lookup and just move on.
Summary: UMO says TB 1.0 is invalid when uploading new extension → Additem must not error for unrecognized GUIDs
*** Bug 290695 has been marked as a duplicate of this bug. ***
Attached patch Proposed Fix (obsolete) (deleted) — Splinter Review
Proposed patch, based on Comment #1 and my own research. Adds a check to see if there's any rows returned matching the guid and skips the app if there's not. This isn't a problem since the code in step2 generates the compatibility list based on recognized GUIDs in the applications table, so if somebody puts in a bogus Firefox GUID by accident, it'll get ignored just like a whole new app would. Also fixed the bogus error messages by unseting $appname at the beginning of the loop and making a null $appname use the guid. I don't have a dev enviroment to test this though, so it *should* be tested before being used on the live site.
Assignee: Bugzilla-alanjstrBugs → bugtrap
Status: NEW → ASSIGNED
Attachment #180987 - Flags: first-review?(mike.morgan)
Do you need to unset them since 2 lines later you set them? Its already established that you found a valid row.
They only get set /if/ # of rows is greater than 0, in the bug before it wasn't getting set as null since the while loop was never entered.
Target Milestone: 1.0 → 1.1
Comment on attachment 180987 [details] [diff] [review] Proposed Fix - patch does not fix problem described - it looks like a GROUP BY AppName fixes the stated problem - need to look at this more and determine what the real source of the problem is -- Wolf, could you get set up with an account on chameleon and hammer on this?
Attachment #180987 - Flags: first-review?(mike.morgan) → first-review-
(In reply to comment #6) > - patch does not fix problem described The patch fixes the problem described on my test install? > - it looks like a GROUP BY AppName fixes the stated problem That makes matters much much worse > - need to look at this more and determine what the real source of the problem > is The cause of the problem is that an invalid GUID returns 0 rows in the query, but the variables still contain the previous application names. This, of course, reports a bogus error message. This patch fixes the bogus error message, and means extensions with GUIDs that we do not support can be added to UMO for the applications that we DO support. What is the actual problem with the patch you found, as my testing shows it works great?
Comment on attachment 180987 [details] [diff] [review] Proposed Fix The patch looks good to me. Colin says the patch works. Please take another look at this.
Attachment #180987 - Flags: first-review- → first-review?(mike.morgan)
Blocks: 290683
Summary: Additem must not error for unrecognized GUIDs → [Submission] Additem must not error for unrecognized GUIDs
Any progress on actually reviewing the attached patch? :-)
after 7 months waiting for review, this simple patch really should be done with by now... morgamic: Is there a plan to review this in the known future, if not, mind bouncing this to somebody who can? Its senseless for a simple change to wait this long to be reviewed... :-)
Comment on attachment 180987 [details] [diff] [review] Proposed Fix - bit-rotted - flawed logic -- you could potentially submit an addon with no valid applications; shouldn't we have at least one valid application?
Attachment #180987 - Flags: first-review?(morgamic) → first-review-
Wolf - this fell through the cracks - thanks for reminding me.
Assignee: bugtrap → morgamic
Status: ASSIGNED → NEW
*** Bug 319074 has been marked as a duplicate of this bug. ***
Dan - do you see support for unrecognized GUIDs as a security risk?
Status: NEW → ASSIGNED
Attached patch Band-aid for a massive head wound. (obsolete) (deleted) — Splinter Review
Patch to require at least one valid GUID, and for all matching GUIDs min and max versions are still enforced. Legacy code still needs to be stripped out, and the entire 800+ line file needs to be rewritten, time permitting.
Attachment #180987 - Attachment is obsolete: true
Attachment #205121 - Flags: first-review?(bugtrap)
Attached patch Forgot -up8N. (deleted) — Splinter Review
Forgot -up8N.
Attachment #205121 - Attachment is obsolete: true
Attachment #205123 - Flags: first-review?
Attachment #205121 - Flags: first-review?(bugtrap)
Attachment #205123 - Flags: first-review? → first-review?(bugtrap)
I'm guessing, from the fact that 319074 has been marked as a duplicate of this bug, that Flock is going to become an "unrecognized" GUID? Could that also be done for Sunbird? That one always causes min/maxAppVersion reject problems too, due to various reasons. What exactly will be the list of "recognized" GUIDs? Thanks.
Flock is a non-Mozilla application, Sunbird is. Non-recognized GUIDs are applications that are not Mozilla projects. The Sunbird version issue will be handled by using 'understandable' versioning as was discussed in that bug.
The application manager says that AMO knows about Flock.
(In reply to comment #18) > Flock is a non-Mozilla application, Sunbird is. Non-recognized GUIDs are > applications that are not Mozilla projects. Flock is a mozilla application, as in built on the Mozilla platform. Why was it removed from "Valid App Versions for Addon Developers" https://addons.mozilla.org/faq.php it is no less valid than Netscape nor Nvu. NVU and Netscape are
This has been checked in and will be pushed this morning.
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Comment on attachment 205123 [details] [diff] [review] Forgot -up8N. Don't want to wait 7 months. :)
Attachment #205123 - Flags: first-review?(bugtrap) → first-review+
Oops. I looked at it and it looked fine. :-) Forgot to mark it though.
Suuure. ;)
Product: addons.mozilla.org → addons.mozilla.org Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: