952284 - Tags set to private comments should not be disclosed to everybody in the bug activity table

Status: RESOLVED FIXED

(Bugzilla :: Creating/Changing Bugs, defect)

Description

[Frédéric Buclin]

In bug 392184, comments 22 to 33 have first been marked as spam and then marked as private (restricted to the insidergroup) to hide them from the UI. But the bug history still shows tags set for these comments.

If a comment is private, all the activity associated with it should be restricted to the insidergroup.

Comment 1

[Daniel Veditz [:dveditz]]

Are there tags other than spam? Doesn't seem too terrible to leak the knowledge that someone thought a particular missing comment was spam. The comment's existence can usually be inferred from discontinuous comment numbers.

Comment 2

[Daniel Veditz [:dveditz]]

Oh, I see--tags are free-form. That's slightly worse, might leak project names or who knows what depending on the bugzilla installation.

Comment 3

[Gervase Markham [:gerv]]

If we did this, it would mean that not only do comments need a "private" flag, but all history entries do as well.

Gerv

Comment 4

[Frédéric Buclin]

(In reply to Gervase Markham [:gerv] from comment #3)
> If we did this, it would mean that not only do comments need a "private"
> flag, but all history entries do as well.

I don't see why. Comments already have that private flag, as well as attachments, which are the only two things you can make private. And all the bug activity are hidden for users not in the insidergroup. So comment tags must respect that too.

Comment 5

[Gervase Markham [:gerv]]

(In reply to Frédéric Buclin from comment #4)
> (In reply to Gervase Markham [:gerv] from comment #3)
> > If we did this, it would mean that not only do comments need a "private"
> > flag, but all history entries do as well.
> 
> I don't see why. 

Because when I am displaying the history and querying the history table, I need to know which entries to not display. Either the history entries can have a "private" flag, or you have to store the comment number along with the history entry for add/remove tags, do a massive JOIN to the comments table, and use the Private flag there. Which you could do, I guess - not sure how that would go performance-wise, though.

Gerv

Comment 6

[David Lawrence [:dkl]]

Attachment: 952284_1.patch

Comment 7

[David Lawrence [:dkl]]

Comment on attachment 8350669 [details] [diff] [review]
952284_1.patch

Gerv mentioned he would not have time to look at this so re-requesting.

Comment 8

[Frédéric Buclin]

Comment on attachment 8350669 [details] [diff] [review]
952284_1.patch

>+            $suppjoins = "LEFT JOIN longdescs

Why LEFT JOIN instead of INNER JOIN?

Comment 9

[Frédéric Buclin]

Comment on attachment 8350669 [details] [diff] [review]
952284_1.patch

>+            $suppwhere = "AND COALESCE(longdescs.isprivate, 0) = 0";

Also, there is no need to call COALESCE(). longdescs.isprivate is always defined (cannot be NULL) and so longdescs.isprivate = 0 will do it.

Comment 10

[David Lawrence [:dkl]]

Attachment: 952284_2.patch

Comment 11

[Frédéric Buclin]

Comment on attachment 8355079 [details] [diff] [review]
952284_2.patch

r=LpSolit

Comment 12

[Frédéric Buclin]

This code isn't in any Bugzilla release (it wasn't in 4.5.1, and 4.5.2 isn't released yet), so the patch can be checked in once it gets approval.

Comment 13

[David Lawrence [:dkl]]

Committing to: bzr+ssh://[e-mail]/bugzilla/trunk
modified Bugzilla/Bug.pm
Committed revision 8849