532500 - Enhancement to how url is sanitized for urldims table

Status: RESOLVED FIXED

(Socorro :: General, task)

Description

[Austin King [:ozten]]

For the urldims table, we want to redefine how we process a crashes's url into the url value of the urldims table.

Currently rules:
urldims.url is everything left of the query string.

New rules:
1) urldims.url is everything left of any of the following characters - ? & = ;
2) if, after rule 1, the urldims.url is longer than N characters, then truncate to N characters

To see example urls:
1) Go to http://crash-stats.stage.mozilla.com/topcrasher/byurl/Firefox/3.5.3
2) Note the following urls are very long

Actual:
http://video.fc2.com/content/%E6%A9%9F%E5%8B%95%E6%88%A6%E5%A3%AB%E3%82%AC%E3%83%B3%E3%83%80%E3%83%A00080%E3%83%9D%E3%82%B1%E3%83%83%E3%83%88%E3%81%AE%E4%B8%AD%E3%81%AE%E6%88%A6%E4%BA%89%20ep1.3%2F4/20080721r7zdh4n2

http://us.mc595.mail.yahoo.com/mc/showFolder;_ylc=X3oDMTBuMHVuMm81BF9TAzM5ODMwMTAxNARhYwNmbHRDb250

http://www.auvito.de/3187/12576/keyword_dachziegel/kategorie.html&ac=au2&utm_source=sem&utm_medium=google&utm_campaign=newkeywords

Expected with N set to 100:

http://video.fc2.com/content/%E6%A9%9F%E5%8B%95%E6%88%A6%E5%A3%AB%E3%82%AC%E3%83%B3%E3%83%80%E3%83%A

http://us.mc595.mail.yahoo.com/mc/showFolder

http://www.auvito.de/3187/12576/keyword_dachziegel/kategorie.html

This is a high priority as malformed (missing the '?' before a query string) or especially long paths may compromise a user's privacy.

Comment 1

[Austin King [:ozten]]

Marking as 1.3, but since it is critical, it can be deployed earlier.

Comment 2

[K Lars Lohn [:lars] [:klohn]]

in rule #1, is the hyphen '-' part of the list or part of the sentence introducing the list?  Since the hyphen is a legitimate part of a domain name, this could be important

Comment 3

[Frank Griswold [:griswolf] [:fgriswold]]

Per IRC with ozten: the cutoff characters are [?&=;]

Comment 4

[Frank Griswold [:griswolf] [:fgriswold]]

Can implement the cutoff characters with almost no problem. Should I assume N defaults to 100? The urldims table has no limit on that field (type TEXT)

Comment 5

[Frank Griswold [:griswolf] [:fgriswold]]

Sending        cron/topCrashesByUrl.py
Sending        database/cachedIdAccess.py
Sending        unittest/database/testCachedIdAccess.py
Transmitting file data ...
Committed revision 1552.

I did not enforce a maximum length for urls, but the code is in place and tested. Needs a config addition that is used by topCrashesByUrl.py. Suggested name for that option: truncateUrlLength

Comment 6

[Michael Morgan [:morgamic]]

Needs a config option, Frank will work on that so we can wrap it up.

Comment 7

[Frank Griswold [:griswolf] [:fgriswold]]

There is a potential issue

What do we want to do about existing urls in table urldims that have length greater than truncateUrlLength? The simplest thing is to ignore the issue: the old long ones are old and long, and the new ones are never > (whatever length).

By leaving the old ones alone, we avoid trouble three ways:
 1: There are something over 200 rows in urldims that would have non-unique (domain,url) if we truncate them to length 100 (88 distinct values). Fixing this is 'hard'
 2: As we begin looking at the truncated values, the old long values will gradually become less important: No extra cost
 3: If we ever want to change the truncation length, we don't have to do a massive update.

I intend to use the 'ignore' option unless I hear otherwise.

Comment 8

[Frank Griswold [:griswolf] [:fgriswold]]

Revision 1654 (on trunk) handles the truncation. 

To have an effect: 
  .../scripts/config/topCrashesByUrlConfig.py must have a new truncateUrlLength Option (copy from topCrashesByUrlConfig.py.dist)
  invoke startTopCrashesByUrl.py with some value for that Option. I have no requirements for what this value should be (100 was used as an example above)

Comment 9

[K Lars Lohn [:lars] [:klohn]]

we need an answer to this question on what the truncate value to be set to.  Who are the stakeholders here?  We need to get this change pushed into staging asap prior to the final push to production.  We cannot make that staging push until I know what to set the value to.  100?

Comment 10

[Austin King [:ozten]]

(In reply to comment #9)
100 sounds good and we can adjust it easily later.

Comment 11

[K Lars Lohn [:lars] [:klohn]]

(04:22:17 PM) ozten: lars: I'm not really the definitive source of requirements, but I'm happy to make something up :)
(04:22:48 PM) lars: who is the definitive source for the answer?
(04:23:20 PM) ozten: I don't think there is one, we're being proactive on this fix
(04:23:42 PM) lars: then  I guess we do make up a value.  How about 100?
(04:23:48 PM) ozten: sounds great
(04:23:51 PM) lars: done