searching for /find bar/ under "get add-ons" generates error

VERIFIED FIXED in mozilla1.9.1b2

Status

()

Toolkit
Add-ons Manager
P2
normal
VERIFIED FIXED
10 years ago
9 years ago

People

(Reporter: mcdavis941 (sporadically reading bugmail), Assigned: mossop)

Tracking

unspecified
mozilla1.9.1b2
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Searching for the /find bar/ add-on in the Add-on Manager's "Get Add-ons" pane generates three of this error:

document.getElementById(id) is null
chrome://mozapps/content/extensions/extension.xml line 387

STR:
1.  Use 20080404 minefield nightly
2.  Open Add-ons Mgr to "Get Add-ons" pane
3.  Enter /find bar/ in the search box and search for it
4.  Errors show up in console

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9pre) Gecko/2008040404 Minefield/3.0pre
(Assignee)

Comment 1

10 years ago
The error message in the console is unrelated.

I think this is related to bug 415269 and may be an AMO issue. The server seems to be unescaping the escaped search terms somewhere along the way leading to an invalid query:

https://services.addons.mozilla.org/en-US/firefox/api/1/search/%2ffind%20bar%2f

Any ideas Laura?

Comment 2

10 years ago
I'm going to pile on a few more strangenesses and inconsistencies. Right now if you do a search for any of the following chars: !#&-_+:

You'll get some results or "no match"

If your search includes any of the following characters, you'll get a "Firefox couldn't retrieve add-ons": ~`@$%^*()=[]\{}|;'",/<>?

We need to be consistent, I thought that the error message was only displayed for connectivity issues. Is the matching due to some regex/XML input filtering? Is this Add-on Mgr code or AMO API?

Also, searching for "-" or "." gives me MANY matches but no results in the list, is that expected behavior?
(Assignee)

Comment 3

10 years ago
(In reply to comment #2)
> I'm going to pile on a few more strangenesses and inconsistencies. Right now if
> you do a search for any of the following chars: !#&-_+:
> 
> You'll get some results or "no match"

That is exactly what the API tells us:

https://services.addons.mozilla.org/en-US/firefox/api/1/search/test!&-_+

> If your search includes any of the following characters, you'll get a "Firefox
> couldn't retrieve add-ons": ~`@$%^*()=[]\{}|;'",/<>?

For these characters the API is returning a completely empty page, no XML to process:

https://services.addons.mozilla.org/en-US/firefox/api/1/search/@find

> We need to be consistent, I thought that the error message was only displayed
> for connectivity issues. Is the matching due to some regex/XML input filtering?
> Is this Add-on Mgr code or AMO API?

The addons manager is behaving as sensibly as possible in this regard I believe. It displays the error when the server returns an invalid response, which means any error pages or a result that isn't XML.

> Also, searching for "-" or "." gives me MANY matches but no results in the
> list, is that expected behavior?

Some of the changes that were made to the API to improve performance involved only including the first 10 search results in the list returned to the client. In these cases it looks like there are indeed a lot of matches, but none of the first 10 are 3.0 compatible. It is I agree not an ideal behaviour, it should improve with bug 419057

https://services.addons.mozilla.org/en-US/firefox/api/1.1/search/-
(Assignee)

Updated

10 years ago
Duplicate of this bug: 439036
Product: Firefox → Toolkit
(Assignee)

Comment 5

10 years ago
The bits from comment 2 and 3 seem fixed now. The original issue still exists and it is the API giving an error response so I believe this is an API issue.
Component: Add-ons Manager → API
Product: Toolkit → addons.mozilla.org
QA Contact: extension.manager → api
Version: Trunk → 3.2

Updated

10 years ago
Assignee: nobody → laura
Target Milestone: --- → 4.0.2

Updated

10 years ago
Priority: -- → P2

Comment 6

10 years ago
Ok, so after much research and banging my head against mod_rewrite, I finally found this is actually caused by an Apache bug.
https://issues.apache.org/bugzilla/show_bug.cgi?id=34602
We can't do much about it at the API level, since Apache chokes on the URL and it never gets any further.

There is a workaround here:
http://us.php.net/manual/en/function.urlencode.php#73641
basically, double urlencode params that contain special characters.

This will have to be worked around at the Addons Manager level, re-assigning.
Assignee: laura → nobody
Component: API → Add-ons Manager
OS: Windows XP → All
Product: addons.mozilla.org → Toolkit
QA Contact: api → extension.manager
Hardware: PC → All
Target Milestone: 4.0.2 → ---
Version: 3.2 → unspecified
(Assignee)

Comment 7

10 years ago
Will it require an API change to unencode the url twice first?

Comment 8

10 years ago
I'll test, but I don't think so.  (If it's going to cause a problem we'll have to do an API version bump, so I hope not.)
(Assignee)

Comment 9

10 years ago
It looks as though it is working fine with just the client side change. However given that we're effectively changing the url passed for a given search we may have to consider revving the api version anyway in case people have been pointing to other services. I haven't heard of this happening so potentially we could ignore it, but imagine the case where apache got fixed, and AMO updated, you'd then be in a position where urls from Firefox 3.1 were encoded too much
(Assignee)

Comment 10

10 years ago
Created attachment 343198 [details] [diff] [review]
patch rev 1

Ok I have increased the API version to 1.2 with this change. AMO already seems to be happy dishing out results on that but can you confirm you are happy with the bump Laura?
Assignee: nobody → dtownsend
Status: NEW → ASSIGNED
Attachment #343198 - Flags: review?(robert.bugzilla)
(Assignee)

Updated

10 years ago
Attachment #343198 - Flags: review?(laura)
Attachment #343198 - Flags: review?(robert.bugzilla) → review+

Comment 11

10 years ago
If you tested this, and it works as is vs servieces.AMO ...the question is if we actually need the version bump?

Comment 12

10 years ago
That would be "services" as in https://services.addons.mozilla.org.  Please excuse my typing.
(Assignee)

Comment 13

10 years ago
(In reply to comment #11)
> If you tested this, and it works as is vs servieces.AMO ...the question is if
> we actually need the version bump?

I am saying we do because we have changed the url the client requests. While it may not make much difference to AMO, it may do to the API service that say Songbird might choose to use. It could be pretty critical for them to know how escaped the request url is.

Comment 14

10 years ago
OK cool - as long as everybody realizes the code doesn't change at all on the API side (how confusing :) )
(Assignee)

Updated

10 years ago
Attachment #343198 - Flags: review?(laura)
(Assignee)

Comment 15

10 years ago
Landed http://hg.mozilla.org/mozilla-central/rev/60cadfb3a231
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.1b2
Verified fix on Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US;
rv:1.9.1.3pre) Gecko/20090819 Shiretoko/3.5.3pre and Mozilla/5.0 (Macintosh; U;
Intel Mac OS X 10.5; en-US; rv:1.9.1.3pre) Gecko/20090819 Shiretoko/3.5.3pre
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.