272163 - private comment identifiers are not really unique under different contexts

Status: RESOLVED INVALID

(Bugzilla :: Creating/Changing Bugs, defect)

Description

[Christian Reis]

Comments are numbered as they are displayed. In a collection of comments, a
private comment will be assigned a number in the order it appears.

If you view the bug as a regular user, however, the collection of comments omit
the private comment, and therefore the comment number assigment will be
inconsistent with the assignment in the private view.

Exemplifying, a bug with 3 comments viewed by an insider displays:

1. public comment A
2. private comment A
3. public comment B

For a public viewer, he will see:

1. public comment A
2. public comment B

The link to comment 3 is broken and the link to comment 2 is inconsistent. 

My suggestion is namespacing private comments by prefixing them with a "P". This
guarantees that at a determined point in time, all links to comments are
consistent (the link to private comment A would be #p1). 

Note that comments that are private and later converted to public, or
vice-versa, can cause a temporal inconsistency. 

I'm marking this as major as it is almost a security issue (well, depending on
the circumstance a public link to a private comment may reveal that a private
comment was made). OTOH a public link to a comment with the ID #p1 also does,
albeit more obviously.

Comment 1

[Marc Schumann [:Wurblzap]]

I cannot reproduce this -- what I'm seeing (on HEAD) is

1. public comment A
3. public comment B

so the links aren't broken.

(This is, though, on a similar level of security-issue-ness, I think).

Comment 2

[Christian Reis]

How odd, the version I'm hacking on must be customized, I hadn't assumed that.
Interesting that it's a different problem, then. Ah, I see here, indeed,
comments.html.tmpl does the process regardless of being an insider.

Do you think this is the way it should be?

Comment 3

[Marc Schumann [:Wurblzap]]

We cannot stop linking to private comments, as little as we can stop anybody
mentioning something confidential in a comment. So we'll have to live with this
kind of leakage, I think.

This said, the numbering gaps are a little annoying. Leakage-wise, too.
I cannot think of a way that covers this and keeps links consistent at the same
time, though.

Should we perhaps mark this INVALID or morph?

Comment 4

[Christian Reis]

Sorry for the red herring, you're right. I should hack the customized version to
include this fix, dammit.