Closed Bug 668687 Opened 10 years ago Closed 10 years ago

postcrash email not working with signatures containing spaces

Categories

(Socorro :: General, task, P1)

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: rhelmer, Assigned: rhelmer)

Details

Attachments

(2 files)

Michael Verdi is preparing some postcrash email campaigns, and discovered that entering signatures containing '*' will always report:

"Sending this email will contact 0 Firefox 4.0 users"

However I can tell from querying the DB directly that this criteria should find 74 email addresses:

"""
breakpad=> select count(*) from reports where signature = 'mozilla::plugins::PPluginIdentifierParent::CreateSharedMemory(unsigned int, mozilla::ipc::SharedMemory::SharedMemoryType, bool, int*)' and version = '4.0' and email != '' and date_processed > '2010-06-01' and date_processed < '2011-06-01';
 count 
-------
    74
(1 row)
"""

STR:

1. Load https://crash-stats.mozilla.com/admin/email
2. Enter the following:
 Product: Firefox
 Versions (Comma separated): 4.0
 Exact signature: mozilla::plugins::PPluginIdentifierParent::CreateSharedMemory(unsigned int, mozilla::ipc::SharedMemory::SharedMemoryType, bool, int*)
 Subject: test1
 Email Body: (unchanged)
 Start Date: 01/06/2010
 End Date: 01/06/2011
3. Click button "Next Step"

Expected result:
"Sending this email will contact 74 Firefox 4.0 users"

Actual result:
"Sending this email will contact 0 Firefox 4.0 users"
Set the milestone to 2.0 because I'd like to get this fixed before 2.1, eg a 2.0.1 patch release.
Status: NEW → ASSIGNED
(In reply to comment #1)
> Set the milestone to 2.0 because I'd like to get this fixed before 2.1, eg a
> 2.0.1 patch release.

I'll just mark this 2.1 instead since we don't have a 2.0.1 milestone, so we can actually find it.
Target Milestone: 2.0 → 2.1
Priority: -- → P1
Summary: postcrash email not working with signatures containing '*' → postcrash email not working with signatures containing spaces
This form field is being sent over GET so use unquote_plus
Attachment #544291 - Flags: review?(chris.lonnen)
So the problem here is just the initial estimate that is given before the user saves the campaign. I think if you went ahead and saved despite the estimate being 0, you'd have a reasonable campaign.

There is a separate service which does the estimates (it does a simpler query and attempts to provide an upper-bound for the number of email addresses that can go out) called emailCampaignVolume which takes the form submission over a GET.

After the user saves the campaign, the emailCampaignCreate service receives all the same data as a POST, and then does a more complex query and stores the campaign and all outgoing email addresses and their status.

It probably doesn't make sense for these two things to be totally separate services, I think they should share a lot more code/query methods/SQL/etc. The estimate doesn't take a lot of the constraints from the spec into account too so it's going to be wrong much of the time. I'll file a separate bug for that.
Comment on attachment 544291 [details] [diff] [review]
unquote signature

This works. Email events are infrequent enough that logging them shouldn't be an issue.
Attachment #544291 - Flags: review?(chris.lonnen) → review+
committed on behalf of rhelmer as r3272.
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Notes for QA:

1. Load https://crash-stats.mozilla.com/admin/email
2. Enter the following:
  Product: Thunderbird
  Versions (Comma separated): 5.0
  Exact signature: UnmarkGrayChildren
  Subject: test1
  Email Body: (unchanged)
  Start Date: 12/07/2011
  End Date: 20/07/2011
3. Click button "Next Step"

Expected result:
"Sending this email will contact 51 Thunderbird 5.0 users who crashed on UnmarkGrayChildren between 12/07/2011 and 20/07/2011"

This matches the results of this query:

SELECT count(distinct email) as total FROM reports        WHERE TIMESTAMP WITHOUT TIME ZONE '2011-07-12 00:00:00' <= reports.date_processed AND              TIMESTAMP WITHOUT TIME ZONE '2011-07-20 23:59:59' > reports.date_processed AND                     product = E'Thunderbird' AND
               version IN (E'5.0') AND 
              signature = E'UnmarkGrayChildren' AND
              email IS NOT NULL AND
              email ~ '.*@.*\.[a-zA-Z]{2,4}';

As we discussed in IRC, there's no need to continue saving the email campaign, since the modified code is just the estimate above.

Please DO NOT send, since the email addresses have not been scrubbed :)
(In reply to comment #7)
> Notes for QA:
> 
> 1. Load https://crash-stats.mozilla.com/admin/email
> 2. Enter the following:
>   Product: Thunderbird
>   Versions (Comma separated): 5.0
>   Exact signature: UnmarkGrayChildren

Le sigh, sorry this signature does not contain a space :) Also, this needs to be done on crash-stats-dev since stage has not been upgraded yet.

I've prepared a new set of instructions:
1. Load https://crash-stats.mozilla.com/admin/email
2. Enter the following:
  Product: Firefox
  Versions (Comma separated): 3.6
  Exact signature: nsChromeTreeOwner::OnLocationChange(nsIWebProgress*, nsIRequest*, nsIURI*)
  Subject: test1
  Email Body: (unchanged)
  Start Date: 13/07/2011
  End Date: 20/07/2011
3. Click button "Next Step"

Expected result:
"Sending this email will contact 6 Firefox 3.6 users who crashed on nsChromeTreeOwner::OnLocationChange(nsIWebProgress*, nsIRequest*, nsIURI*) between 13/07/2011 and 20/07/2011"

This matches the results of this query:

SELECT count(distinct email) as total FROM reports        WHERE TIMESTAMP WITHOUT TIME ZONE '2011-07-13 00:00:00' <= reports.date_processed AND              TIMESTAMP WITHOUT TIME ZONE '2011-07-20 23:59:59' > reports.date_processed AND
              product = E'Firefox' AND
               version IN (E'3.6') AND
              signature = E'nsChromeTreeOwner::OnLocationChange(nsIWebProgress*, nsIRequest*, nsIURI*)' AND
              email IS NOT NULL AND
              email ~ '.*@.*\.[a-zA-Z]{2,4}';

Please do not save or send this campaign :)
(In reply to comment #8)
> 
> Le sigh, sorry this signature does not contain a space :) Also, this needs
> to be done on crash-stats-dev since stage has not been upgraded yet.
> 
> I've prepared a new set of instructions:
> 1. Load https://crash-stats.mozilla.com/admin/email

Actually above URL should be https://crash-stats-dev.allizom.org/admin/email

> 2. Enter the following:
>   Product: Firefox
>   Versions (Comma separated): 3.6
>   Exact signature: nsChromeTreeOwner::OnLocationChange(nsIWebProgress*,
> nsIRequest*, nsIURI*)
>   Subject: test1
>   Email Body: (unchanged)
>   Start Date: 13/07/2011
>   End Date: 20/07/2011
> 3. Click button "Next Step"
> 
> Expected result:
> "Sending this email will contact 6 Firefox 3.6 users who crashed on
> nsChromeTreeOwner::OnLocationChange(nsIWebProgress*, nsIRequest*, nsIURI*)
> between 13/07/2011 and 20/07/2011"
> 
> This matches the results of this query:
> 
> SELECT count(distinct email) as total FROM reports        WHERE TIMESTAMP
> WITHOUT TIME ZONE '2011-07-13 00:00:00' <= reports.date_processed AND       
> TIMESTAMP WITHOUT TIME ZONE '2011-07-20 23:59:59' > reports.date_processed
> AND
>               product = E'Firefox' AND
>                version IN (E'3.6') AND
>               signature =
> E'nsChromeTreeOwner::OnLocationChange(nsIWebProgress*, nsIRequest*,
> nsIURI*)' AND
>               email IS NOT NULL AND
>               email ~ '.*@.*\.[a-zA-Z]{2,4}';
> 
> Please do not save or send this campaign :)
Verified FIXED; thanks for the steps, Rob!
Status: RESOLVED → VERIFIED
OS: Linux → All
Hardware: x86_64 → All
Component: Socorro → General
Product: Webtools → Socorro
You need to log in before you can comment on or make changes to this bug.