1371428 - ' turned into '' in signature

Status: RESOLVED FIXED

(Socorro :: Signature, task)

Description

[Andrew McCreight [:mccr8]]

This is a minor issue but I figured I'd file it. There are some crashes (such as bp-a369bb76-c381-4688-bf43-d5db10170608 ) that have a top frame like this:
`vector constructor iterator'(void*, unsigned __int64, unsigned __int64, void* (*)(void*))

However, the signature they end up with is [@ `vector constructor iterator'' ]. It seems odd that the ' becomes ''.

Comment 1

[Will Kahn-Greene [:willkg] ET needinfo? me]

Signature generation cli says this:

app@79c52023f393:/app$ python -m socorro.signature -v a369bb76-c381-4688-bf43-d5db10170608
Original: `vector constructor iterator''
New:      `vector constructor iterator''
Same?:    True
Notes:    (2)
          SignatureGenerationRule:  -> `vector constructor iterator''
          SigTrim: `vector constructor iterator'' -> `vector constructor iterator''


So it's happening in the SignatureGenerationRule.

I traced it down to this:

https://github.com/mozilla-services/socorro/blob/1443fd5a0226dc860fe2a3ad76a62db902ec0c6f/socorro/signature/signature_utilities.py#L70

That comes from this commit:

https://github.com/mozilla-services/socorro/commit/374ee1b07382b213718e4513180d21e2782d8ee1

That seems to be related to bug #701002, but I don't see any explanation for why single quotes need to be escaped in this manner.

Since that commit dates back to 2012 or so, I'm inclined to not change this now unless there's a compelling reason to do so.

Comment 2

[Andrew McCreight [:mccr8]]

I guess that's fine. I doubt anybody will try looking up these weird quoted signatures in the search box on crash-stats.

Comment 3

[Will Kahn-Greene [:willkg] ET needinfo? me]

Putting this in the signature component.

Comment 4

[Will Kahn-Greene [:willkg] ET needinfo? me]

I went back through "git annotate" and the commits involved and looked at the surrounding code and the related bugs and I can't figure out why that code was added. We wiped the crash data, so there are like 3500 or so crashes that are affected by this right now. Seems like a good time to nix the quoting and then we can reprocess all the affected crashes.

Grabbing this to do now.

Comment 5

[Will Kahn-Greene [:willkg] ET needinfo? me]

Aha! Seems like it was really added in this commit:

https://github.com/mozilla-services/socorro/commit/277f56436bfc27121f789f54e1146aa832f32362

That commit has this bit in the message:

"""
At the same time time, I backported the 1.8 signature
generation class to 1.7.7 and wrote unit tests.
"""

I can't find any evidence of the 1.8 signature generation class or the commits that led up to that. List of bugs targeted for 1.8 and 1.8.1 is here:

https://bugzilla.mozilla.org/buglist.cgi?list_id=13962607&resolution=FIXED&resolution=INVALID&resolution=WONTFIX&resolution=WORKSFORME&query_format=advanced&target_milestone=1.8&target_milestone=1.8.1&product=Socorro

Maybe this came along with the PHP -> Python rewrite and it harks back to the PHP days?

Anyhow, I'm feeling confident that whatever circumstances led us to convert ' to '' in signatures that are no longer relevant and so I'm going to nix this.

Comment 6

[Will Kahn-Greene [:willkg] ET needinfo? me]

Attachment: pr 4287: fixes bug 1371428 - nix quotes

Comment 7

[Andrew McCreight [:mccr8]]

Thanks!

Comment 8

[Will Kahn-Greene [:willkg] ET needinfo? me]

Lars said this:

<lars> I know the answer to why ' is changed to ''
<lars> the doubling of a single quote goes all the way back to 2009 and the Postgres datastore.  In that time, inserts into a partitioned table disallowed bound variables - signatures had to be literals in SQL statements.  Doubling a signal quote was the PG way of escaping them.
<lars> in fact, here's the original commit that introduced it: f165094ef5185e10d537ba497d9ff14e708ae62a

That's this one: https://github.com/mozilla-services/socorro/commit/f165094ef5185e10d537ba497d9ff14e708ae62a

That makes a ton of sense. I think we're fine to remove it. The processor and saving to postgres work fine without this now.

Comment 9

[[github robot]]

Commits pushed to master at https://github.com/mozilla-services/socorro

https://github.com/mozilla-services/socorro/commit/3d8978c0115d19bf2af52478e485e98a6ce67dea
fixes bug 1371428 - nix ' to '' in signature generation

For years, there's been a bit of code to "escape single quotes" in
Java and C signatures. As near as I can tell, it predates the current
era of Socorro requirements and was likely added for some reason that
is no longer relevant.

One consequence is that it makes searching for frames harder, so this
removes that behavior.

https://github.com/mozilla-services/socorro/commit/1c0dcaedea738acc6d04f097a10d1afa78c84c29
Merge pull request #4287 from willkg/1371428-nix-quotes

fixes bug 1371428 - nix quotes