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

VERIFIED FIXED

Status

()

Core
Networking
VERIFIED FIXED
14 years ago
13 years ago

People

(Reporter: Beth Skwarecki, Assigned: timeless)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: checkwin checklinux, URL)

Attachments

(2 attachments, 1 obsolete attachment)

4.92 KB, text/plain
Details
5.27 KB, patch
Darin Fisher
: review+
Darin Fisher
: superreview+
Details | Diff | Splinter Review
(Reporter)

Description

14 years ago
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.
-> networking
Assignee: file-handling → darin
Component: File Handling → Networking
QA Contact: ian → benc
Sounds like a general issue in 
netwerk/streamconv/converters/nsTXTToHTMLConv.cpp

Comment 3

14 years ago
I can't confirm, behind proxy.

Would be fixed by bug 171150 in any case.
Summary: angle brackets in gopher pages are not properly escaped/displayed → [nsTXTToHTMLConv] angle brackets in gopher pages are not properly escaped/displayed
(Assignee)

Comment 4

14 years ago
Created attachment 147483 [details]
testcase
Assignee: darin → timeless
Status: UNCONFIRMED → ASSIGNED
(Assignee)

Comment 5

14 years ago
Created attachment 147525 [details] [diff] [review]
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.
(Assignee)

Updated

14 years ago
Attachment #147525 - Flags: superreview?(darin)
Attachment #147525 - Flags: review?(darin)

Comment 6

14 years ago
Comment on attachment 147525 [details] [diff] [review]
Pass testcase

what's up with the STOP_CALLS_ODA stuff?  /me no like used #ifdef's.
Attachment #147525 - Flags: superreview?(darin)
Attachment #147525 - Flags: superreview-
Attachment #147525 - Flags: review?(darin)
Attachment #147525 - Flags: review-
(Assignee)

Comment 7

14 years ago
Created attachment 148073 [details] [diff] [review]
patch w/o ifdef
Attachment #147525 - Attachment is obsolete: true
(Assignee)

Updated

14 years ago
Attachment #148073 - Flags: superreview?(darin)
Attachment #148073 - Flags: review?(darin)

Comment 8

14 years ago
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.
Attachment #148073 - Flags: superreview?(darin)
Attachment #148073 - Flags: superreview+
Attachment #148073 - Flags: review?(darin)
Attachment #148073 - Flags: review+
(Assignee)

Comment 9

14 years ago
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.
(Assignee)

Updated

14 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 14 years ago
Resolution: --- → FIXED

Comment 10

13 years ago
V/fixed:
Mozilla 1.8a4, Mac OS X.
Status: RESOLVED → VERIFIED
Whiteboard: checkwin checklinux
You need to log in before you can comment on or make changes to this bug.