242110 - [nsTXTToHTMLConv] angle brackets in gopher pages are not properly escaped/displayed

Status: VERIFIED FIXED

(Core :: Networking, defect)

Description

[Beth Skwarecki]

User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.6) Gecko/20040206 Firefox/0.8
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8a) Gecko/20040428

When text files are fetched with the gopher:// protocol, they are converted to
HTML. 

If the file contains angle brackets (in this example, there are angle brackets
surrounding IRC nicknames), these brackets are not converted to HTML entities,
and so are interpreted as tags and not displayed. 

Angle brackets (and possibly other characters) should be converted so that they
display properly.

Reproducible: Always
Steps to Reproduce:
1. Go to
gopher://gopher.quux.org/0/Humor%20and%20Fun/Fortune%20Databases/Full%20Database:%20IRC:%20Debian%20Hackers
2. Notice the lack of IRC nicknames in angle brackets, for example, <james>
3. View source. The brackets are there, unescaped 

Actual Results:  
Angle brackets that are present in the text do not show up in the rendered page.

Expected Results:  
The angle brackets should have been encoded as &lt; and &gt; so that they show
up properly.

Tested in the latest nightly build for OS X, reproduced in several mozilla-oid
browsers on OS X and Linux, presumably the bug exists on all platforms.

Comment 1

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

-> networking

Comment 2

[Boris Zbarsky [:bzbarsky]]

Sounds like a general issue in 
netwerk/streamconv/converters/nsTXTToHTMLConv.cpp

Comment 3

[Ben Bucksch (:BenB)]

I can't confirm, behind proxy.

Would be fixed by bug 171150 in any case.

Comment 4

[timeless]

Attachment: testcase

Comment 5

[timeless]

Attachment: Pass testcase

Since no code actually exercised the !prepend case, it was fairly broken.

CatHTML's !prepend case was entirely broken: The call to Cut passed an index
instead of a length. It also didn't calculate cursor.

ODA didn't pick a sane choice for front/back in the !prepend case.

I added instances which actually used !prepend: [<>&].

ODA was leaking buffer in various failure cases, so I converted it to an
autobuffer.

Comment 6

[Darin Fisher]

Comment on attachment 147525 [details] [diff] [review]
Pass testcase

what's up with the STOP_CALLS_ODA stuff?  /me no like used #ifdef's.

Comment 7

[timeless]

Attachment: patch w/o ifdef

Comment 8

[Darin Fisher]

Comment on attachment 148073 [details] [diff] [review]
patch w/o ifdef

>Index: nsTXTToHTMLConv.cpp

>+    nsAutoPtr<char> buffer;
>+        buffer = (char*)nsMemory::Alloc(aCount+1);

I think you need to use operator new instead of nsMemory::Alloc
since nsAutoPtr::~nsAutoPtr calls operator delete.


>     convToken *token = new convToken;
>     if (!token) return NS_ERROR_OUT_OF_MEMORY;
>+    token->prepend = PR_FALSE;
>+    token->token.Assign(NS_LITERAL_STRING("<"));
>+    token->modText = NS_LITERAL_STRING("&lt;");

this code might be cleaner if you implemented a constructor on convToken
that takes as its argument the values you are setting here.


r+sr=darin with at least the first change.

Comment 9

[timeless]

mozilla/netwerk/streamconv/converters/nsTXTToHTMLConv.cpp	 1.27

Sorry for the delay. I wanted to use 
    nsAutoArrayPtr<char> buffer(new char[aCount+1]);

Using nsAutoPtr results in a similar FMM since the constructor was new[] and you
need to pair it w/ delete[], not delete.

Using assignment instead of initialization results in gcc295 deleting the
temporary at the time it initializes the array. This isn't a good thing because
you're using a deleted object for the life of the function and you crash at
function exit.

But I was stuck on this second problem for a while and didn't figure it out
until biesi poked me about it today.

Comment 10

[benc]

V/fixed:
Mozilla 1.8a4, Mac OS X.