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)
Tracking
(Not tracked)
RESOLVED
FIXED
1.1
People
(Reporter: Bugzilla-alanjstrBugs, Assigned: morgamic)
References
()
Details
Attachments
(1 file, 2 obsolete files)
(deleted),
patch
|
morgamic
:
first-review+
|
Details | Diff | Splinter Review |
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. ***
Comment 3•20 years ago
|
||
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.
Updated•20 years ago
|
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.
Comment 5•20 years ago
|
||
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.
Assignee | ||
Comment 6•20 years ago
|
||
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-
Comment 7•20 years ago
|
||
(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)
Summary: Additem must not error for unrecognized GUIDs → [Submission] Additem must not error for unrecognized GUIDs
Comment 9•19 years ago
|
||
Any progress on actually reviewing the attached patch? :-)
Comment 10•19 years ago
|
||
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... :-)
Assignee | ||
Comment 11•19 years ago
|
||
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-
Assignee | ||
Comment 12•19 years ago
|
||
Wolf - this fell through the cracks - thanks for reminding me.
Updated•19 years ago
|
Assignee: bugtrap → morgamic
Status: ASSIGNED → NEW
Assignee | ||
Comment 13•19 years ago
|
||
*** Bug 319074 has been marked as a duplicate of this bug. ***
Assignee | ||
Comment 14•19 years ago
|
||
Dan - do you see support for unrecognized GUIDs as a security risk?
Status: NEW → ASSIGNED
Assignee | ||
Comment 15•19 years ago
|
||
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)
Assignee | ||
Comment 16•19 years ago
|
||
Forgot -up8N.
Attachment #205121 -
Attachment is obsolete: true
Attachment #205123 -
Flags: first-review?
Attachment #205121 -
Flags: first-review?(bugtrap)
Assignee | ||
Updated•19 years ago
|
Attachment #205123 -
Flags: first-review? → first-review?(bugtrap)
Comment 17•19 years ago
|
||
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.
Assignee | ||
Comment 18•19 years ago
|
||
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.
Reporter | ||
Comment 19•19 years ago
|
||
The application manager says that AMO knows about Flock.
Comment 20•19 years ago
|
||
(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
Assignee | ||
Comment 21•19 years ago
|
||
This has been checked in and will be pushed this morning.
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 22•19 years ago
|
||
Comment on attachment 205123 [details] [diff] [review]
Forgot -up8N.
Don't want to wait 7 months. :)
Attachment #205123 -
Flags: first-review?(bugtrap) → first-review+
Comment 23•19 years ago
|
||
Oops. I looked at it and it looked fine. :-) Forgot to mark it though.
Assignee | ||
Comment 24•19 years ago
|
||
Suuure. ;)
Updated•9 years ago
|
Product: addons.mozilla.org → addons.mozilla.org Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•