753382 - Escaping problem with rather complex signature

Status: RESOLVED FIXED

(Socorro :: Webapp, task)

Description

[Robert Kaiser]

I ran across this signature in Firefox 13.0b2:

base::LazyInstance<base::ThreadLocalPointer<NotificationService>, base::DefaultLazyInstanceTraits<base::ThreadLocalPointer<NotificationService> > >::Pointer()


Now, calling up report/list for this shows 1) an interestingly broken header and 2) no matching crash reports, even though I know there are some:
https://crash-stats.mozilla.com/report/list?signature=base%3A%3ALazyInstance%3Cbase%3A%3AThreadLocalPointer%3CNotificationService%3E%2C%20base%3A%3ADefaultLazyInstanceTraits%3Cbase%3A%3AThreadLocalPointer%3CNotificationService%3E%20%3E%20%3E%3A%3APointer%28%29

But there's more:
https://crash-stats.mozilla.com/topcrasher/byversion/Firefox/13.0b2/1/browser shows this at #78 today and displays "base::LazyInstance, base::DefaultL" in the visible area of the table, the selection source tells me that "base::LazyInstance<base::threadlocalpointer<notificationservice>, base::DefaultL</base::threadlocalpointer<notificationservice>" is the innerHTML of that link (the <a> tag and its attributes, esp. the href and title are correct, though)!

The code in /query is showing it correctly in the UI, see https://crash-stats.mozilla.com/query/query?product=Firefox&version=Firefox%3A13.0b2&query_search=signature&query_type=contains&query=LazyInstance&do_query=1 for example.

In both TCBS and /query, the linked /report/list fails the same way as the example one above, though.

Comment 2

[Robert Kaiser]

When this is being fixed, we should check the bug 759541 case as well.

Comment 3

[Schalk Neethling [:espressive]]

Sent pull request :: https://github.com/mozilla/socorro/pull/685

Comment 4

[Schalk Neethling [:espressive]]

Wrong bug for the pull request

Comment 5

[Schalk Neethling [:espressive]]

rhelmer/adrian Is this something that has also been resolved in the rewrite?

Comment 6

[Robert Helmer [:rhelmer]]

(In reply to Schalk Neethling [:espressive] from comment #5)
> rhelmer/adrian Is this something that has also been resolved in the rewrite?

I *think* django's urlencode filter does what we want (it's basically python urllib2.quote), it has worked fine in my testing so far.

We should get some tests in for the particularly pathological signatures, like the ones mentioned in this bug. I've filed bug 792087 to cover that.

Comment 7

[Schalk Neethling [:espressive]]

rhelmer, so we can dupe this one against that one? Or make this one a dependency?

Comment 8

[Laura Thomson :laura]

Closing. File bugs for any new problematic signatures.

Comment 9

[Jesse Ruderman]

I see signatures containing "<" in both the top crashers page and the query results page, so I think this has been fixed.

I tried to run the HTML validator to be sure, but bug 910062 made it hard.

Comment 10

[Robert Helmer [:rhelmer]]

(In reply to Jesse Ruderman from comment #9)
> I see signatures containing "<" in both the top crashers page and the query
> results page, so I think this has been fixed.
> 
> I tried to run the HTML validator to be sure, but bug 910062 made it hard.

Hmm I wonder if bug 778063 helped? 

Jessie, would you mind testing against https://crash-stats.allizom.org ?

Comment 11

[Jesse Ruderman]

You mean bug 788063. Yes, it helped.

https://crash-stats.allizom.org/topcrasher/products/Firefox/versions/26.0a1?days=7 passes validator.nu :)

Comment 12

[Robert Helmer [:rhelmer]]

(In reply to Jesse Ruderman from comment #11)
> You mean bug 788063. Yes, it helped.

I did indeed
 
> https://crash-stats.allizom.org/topcrasher/products/Firefox/versions/26.
> 0a1?days=7 passes validator.nu :)

\o/