175324 - Hang in search (lxr)

Status: RESOLVED FIXED

(SeaMonkey :: Search, defect)

Description

[Frank Belew]

User-Agent:       Mozilla/5.0 Galeon/1.2.6 (X11; Linux i686; U;) Gecko/20021016
Build Identifier: Mozilla/5.0 (X11; Linux i686; U;) Gecko/20021016

Mozilla hangs when trying to search for \*\*\* in lxr text search


Reproducible: Always

Steps to Reproduce:
1> Go to lxr.mozilla.org/Seamonkey
2> Enter   \*\*\*    into Text Search
3> Click search, or hit enter
4> Watch mozilla redirect to search URL, and hang

Actual Results:  
Mozilla hangs


Expected Results:  
Completed the search

Comment 1

[Christian :Biesinger (don't email me, ping me on IRC)]

confirming. seeing this as well, on a cvs build from today. no duplicates found.

Comment 2

[Andrew Schultz]

Attachment: stack

Comment 3

[Andrew Schultz]

==> Search

Comment 4

[Andrew Schultz]

http://lxr.mozilla.org/mozilla/source/xpfe/components/search/src/nsInternetSearchService.cpp#5326
resultItemStart is 1446
nsString_Find(resultItemEndStr...) returns -1, so gets reset to stopIndex
(1391), which is less than resultItemStart

bad things then happen when trying to create resultItem.
probably uncovered by bug 120712

Comment 5

[Erich 'Ricky' Iseli]

I believe this is a duplicate of bug 174661. What do you think?

Comment 6

[Andrew Schultz]

Attachment: patch

patch that fixes the hang.

rjc: is this the right thing to do?
end-start<2 would be simpler but less clear.
do we still need to check resultItem for IsEmpty?

Comment 7

[Andrew Schultz]

*** Bug 174661 has been marked as a duplicate of this bug. ***

Comment 8

[David :Bienvenu]

can we get some action on this bug/fix? I see this hang a lot, and it can be on
normal searches.

Comment 9

[Erich 'Ricky' Iseli]

*** Bug 179994 has been marked as a duplicate of this bug. ***

Comment 10

[Erich 'Ricky' Iseli]

*** Bug 148134 has been marked as a duplicate of this bug. ***

Comment 11

[Erich 'Ricky' Iseli]

*** Bug 177674 has been marked as a duplicate of this bug. ***

Comment 12

[Erich 'Ricky' Iseli]

Changing summary to reflect the actual problem. Old summary: "hang when
searching for "\*\*\*" in lxr"

Comment 13

[Rainer Bielefeld]

For my Mozilla Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.2b) Gecko/20021103
the duplicate NETSCAPE button is caused by the files 
SBWeb_01.gif
SBWeb_01.src
Would be interesting to know what phshes these files into my searchplugins from
time to time!

Comment 14

[Rainer Bielefeld]

Attachment: Trouble causing file in searchplugins

see #13

Comment 15

[Rainer Bielefeld]

I always get the message
! You did not specify a file to attach" 
when I try to create my second att.
So please have a look to 
http://home.t-online.de/home/Bielefeld__Buss_oHG/mozilla/SBWeb_01.src

Comment 16

[Andrew Schultz]

Rainer: I don't know why you're getting two Netsacpe icons, but it's not this bug. 
resummarizing a bit

Comment 17

[Simon Fraser [no longer active]]

Stack of hang from bug 179994:

271 nsAppShellService::Run()
271 nsAppShell::Run()
271 nsMacMessagePump::DoMessagePump()
271 nsMacMessagePump::DispatchEvent(int, EventRecord*)
271 Repeater::DoRepeaters(EventRecord const&)
271 nsMacNSPREventQueueHandler::RepeatAction(EventRecord const&)
271 nsMacNSPREventQueueHandler::ProcessPLEventQueue()
271 nsEventQueueImpl::ProcessPendingEvents()
271 PL_ProcessPendingEvents
271 PL_HandleEvent
271 nsARequestObserverEvent::HandlePLEvent(PLEvent*)
271 nsOnStopRequestEvent::HandleEvent()
271 nsHttpChannel::OnStopRequest(nsIRequest*, nsISupports*, unsigned)
271 InternetSearchDataSource::OnStopRequest(nsIRequest*, nsISupports*, unsigned)
271 InternetSearchDataSource::webSearchFinalize(nsIChannel*,
nsIInternetSearchContext*)
271 InternetSearchDataSource::ParseHTML(nsIURI*, nsIRDFResource*,
nsIRDFResource*, unsigned short const*, int)
271 nsAutoString::nsAutoString[in-charge](unsigned short const*, int)
271 nsAutoString::nsAutoString[unified](unsigned short const*, int)
271 nsAString::do_AppendFromElementPtrLength(unsigned short const*, unsigned)
271 nsAString::do_AppendFromReadable(nsAString const&)
271 nsAString::UncheckedAppendFromReadable(nsAString const&)
271 nsString::SetLength(unsigned)
271 nsString::SetCapacity(unsigned)
271 nsStrPrivate::GrowCapacity(nsStr&, unsigned)
271 nsStrPrivate::GrowCapacity(nsStr&, unsigned) [STACK TOP]

I bet a debug build would assert here.

Comment 18

[jag (Peter Annema)]

271 InternetSearchDataSource::ParseHTML(nsIURI*, nsIRDFResource*,
nsIRDFResource*, unsigned short const*, int)
271 nsAutoString::nsAutoString[in-charge](unsigned short const*, int)

http://lxr.mozilla.org/seamonkey/source/xpfe/components/search/src/nsInternetSearchService.cpp#5353


nsAutoString resultItem(&htmlPage[resultItemStart],
                        resultItemEnd - resultItemStart - 1);

I suspect |resultItemEnd - resultItemStart - 1| is negative in this case,
turning into a large number (the nsAutoString constructor takes PRInt32, and
calls Append, inherited from nsAString, which takes a PRUInt32). I don't think
we actually assert on this right now, I can add that.

rjc, could |resultItemEnd - resultItemStart - 1| ever be negative? If so, would
a simple guard against negative lengths suffice?

Comment 19

[Andrew Schultz]

jag: see comment 4 and comment 6

Comment 20

[Robert John Churchill]

Comment on attachment 103484 [details] [diff] [review]
patch

r=rjc

Comment 21

[Robert John Churchill]

Comment on attachment 103484 [details] [diff] [review]
patch

r=rjc

Comment 22

[Andrew Schultz]

Comment on attachment 103484 [details] [diff] [review]
patch

jag, could you sr=?

Comment 23

[jag (Peter Annema)]

Comment on attachment 103484 [details] [diff] [review]
patch

Missed this patch completely :-)

Yeah, that'd do the trick. What would you think about:

  if (resultItemEnd - resultItemStart - 1 <= 0)

I think it's a little clearer than the < 1 test. sr=jag with and without this
stylistic change.

Comment 24

[Andrew Schultz]

Attachment: patch using <=0

<=0 sounds good to me

Comment 25

[Andrew Schultz]

Comment on attachment 106424 [details] [diff] [review]
patch using <=0

carrying forward r=rjc and sr=jag

rjc: could you check this in?  thanks.

Comment 26

[Robert John Churchill]

Fix checked in.

Comment 27

[Robert John Churchill]

.