Closed Bug 519118 Opened 10 years ago Closed 10 years ago

should be able to copy newsgroup link from message header

Categories

(Thunderbird :: Mail Window Front End, enhancement)

enhancement
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 3.0rc1

People

(Reporter: mkmelin, Assigned: mkmelin)

Details

(Whiteboard: [has l10n impact])

Attachments

(1 file)

No description provided.
Attached patch proposed fixSplinter Review
Bug 35689 added the ability to copy newsgroup name. This patch lets the user copy the newsgroup URL too.
Attachment #403126 - Flags: ui-review?(clarkbw)
Attachment #403126 - Flags: review?(dmose)
Comment on attachment 403126 [details] [diff] [review]
proposed fix

In general, this looks good.  However, from a security standpoint, Newsgroups originally comes from an untrusted source (the message).  Now, if there are no bugs in the rest of our code, we'll be fine constructing the URL using strings like this.  However, it's better to have defense in depth.  Ideally, we'd build up the URL object using nsIURI attribute setters on an nsStandardURL object so that it enforces all the right escaping.  Assuming nsStandardURL supports that mode of use, please do that, and request review from again.  If not, please file a bug on nsStandardURL and we'll live with the string construction for now, so r+ing now for that case.
Attachment #403126 - Flags: review?(dmose) → review+
er, "review from me again"
Whiteboard: [has l10n impact]
Comment on attachment 403126 [details] [diff] [review]
proposed fix

looks ok
Attachment #403126 - Flags: ui-review?(clarkbw) → ui-review+
changeset:   3944:38968552494e
http://hg.mozilla.org/comm-central/rev/38968552494e

->FIXED

Bug 519118 filed about building it in a safer way. I'll have to dig in to it a bit.
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Comment on attachment 403126 [details] [diff] [review]
proposed fix

Whops, meant to a-tb3 this myself too.
Attachment #403126 - Flags: approval-thunderbird3+
(In reply to comment #5)
> Bug 519118 filed about building it in a safer way. I'll have to dig in to it a
> bit.

hmm, that's this bug :)
Ah, that was bug 519252.
You need to log in before you can comment on or make changes to this bug.