758407 - depends on bug changed state shows real subject for security bugs

Status: RESOLVED FIXED

(bugzilla.mozilla.org :: General, defect)

Description

[Andrew McCreight [:mccr8]]

I see this occasionally.

Do not reply to this email. You can add comments to this bug at
https://bugzilla.mozilla.org/show_bug.cgi?id=349611

Bug 349611 depends on bug 758398, which changed state.

Bug 758398 Summary: [[actual summary was included here]]

https://bugzilla.mozilla.org/show_bug.cgi?id=758398

          What    |Old Value                   |New Value
----------------------------------------------------------------------------
            Status|NEW                         |RESOLVED
        Resolution|                            |DUPLICATE

bug 349611 is not a secure bug, bug 758398 is.

Comment 1

[:glob ✱]

the summary should be displayed if you have access to that bug.

are you reporting that the email wasn't encrypted?

Comment 2

[Andrew McCreight [:mccr8]]

I don't have an encryption key set up, so that came through in plain text.  When somebody adds a secure bug as a dependency, I think it just says [Secure Bug] or something for the summary, but when the dependency is closed, it gives the summary.  So at least that is inconsistent.  Maybe that is intentional.

Comment 3

[:glob ✱]

i think the best thing to do here is to add the following x-headers to bugmail:

  X-Bugzilla-ID:
  X-Bugzilla-Referenced-IDs:

the securemail ext can use the list of referenced ids to determine if the mail should be sent encrypted.

Comment 4

[Gervase Markham [:gerv]]

Or even one header, where the first number is the bug's ID, and any subsequent ones are referenced?

This would allow us to remove the hack where we extract the bug number from the subject.

Gerv

Comment 5

[:glob ✱]

(In reply to Gervase Markham [:gerv] from comment #4)
> Or even one header, where the first number is the bug's ID, and any
> subsequent ones are referenced?

i think it would be clearer to have separate headers.  should be able to push the x-bugzilla-id header upstream.

> This would allow us to remove the hack where we extract the bug number from
> the subject.

indeed, although we'll still have to have the subject kludge in as a fall-back.

Comment 6

[:glob ✱]

ah, we can do this easier if we make the dependency change a referenced bug, and let the existing bugmail_referenced_bugs hook deal with sanitisation.

Comment 7

[:glob ✱]

Attachment: patch v1

here we go.

this slightly changes the output of bugmail, but i think it's for the better as it'll now be consistent.

the bug summary is still being selected with the query but is now unused; i left it in to minimise the diff from upstream.

Comment 8

[David Lawrence [:dkl]]

Comment on attachment 627879 [details] [diff] [review]
patch v1

Review of attachment 627879 [details] [diff] [review]:
-----------------------------------------------------------------

r=dkl

::: Bugzilla/BugMail.pm
@@ -264,4 @@
>                  $lastbug = $depbug;
>                  $thisdiff =
>                    "\nBug $id depends on bug $depbug, which changed state.\n\n" .
> -                  "Bug $depbug Summary: $summary\n" .

If summary is no longer needed, we can remove it from the SQL and get rid of $summary altogether. Technically doesn't hurt to leave it in but since we are already making changes to core code, it does make the code cleaner. Can fix on checkin if you agree.

Comment 9

[:glob ✱]

Committing to: bzr+ssh://[e-mail]/bmo/4.0/
modified Bugzilla/BugMail.pm
Committed revision 8198.