Last Comment Bug 704722 - Incorrect regexp in OpenSearch
: Incorrect regexp in OpenSearch
Product: MailNews Core
Classification: Components
Component: Search (show other bugs)
: unspecified
: All All
-- normal (vote)
: Thunderbird 12.0
Assigned To: Mark Banner (:standard8)
Depends on:
  Show dependency treegraph
Reported: 2011-11-22 18:59 PST by Daniel Veditz [:dveditz]
Modified: 2012-01-24 14:35 PST (History)
3 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---

The fix (656 bytes, patch)
2012-01-24 14:23 PST, Mark Banner (:standard8)
squibblyflabbetydoo: review+
standard8: approval‑comm‑aurora+
standard8: approval‑comm‑beta+
Details | Diff | Splinter Review

Description User image Daniel Veditz [:dveditz] 2011-11-22 18:59:08 PST
Minor RegExp issue in the webSearchTab.js code that tries to validate clicked links come from Google search requests.

A regexp of "\/url?" will match "/ur"-anything because the question mark makes the "l" optional. What you really want there is  /^\/url\?/.test()

Not really a big deal since you're also checking the host, unlikely that Google would try to fool Thunderbird for malicious purposes. If anything you'll match too much and then send legit intra-Google URLs to the external browser.
Comment 1 User image Jim Porter (:squib) 2011-12-05 21:46:32 PST
It seems that Google doesn't use redirects for its search results anymore (nor does Yahoo), so we should just remove this. We need to keep the special-casing for the Yahoo host name though, since image search is on a different subdomain.
Comment 2 User image Ludovic Hirlimann [:Usul] 2012-01-19 08:25:00 PST
Do we need to fix this before we release 10.0 ?
Comment 3 User image Mark Banner (:standard8) 2012-01-24 14:23:30 PST
Created attachment 591270 [details] [diff] [review]
The fix

As discussed on irc, this should fix it so that just search? urls stay within google (assuming the pre & post hosts are the same).
Comment 4 User image Jim Porter (:squib) 2012-01-24 14:24:38 PST
Comment on attachment 591270 [details] [diff] [review]
The fix

Looks good, as discussed on IRC.

Note You need to log in before you can comment on or make changes to this bug.