Bug 23889 - Negative visit counts stored in History.plist aren't corrected
Summary: Negative visit counts stored in History.plist aren't corrected
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: History (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.5
: P2 Normal
Assignee: John Sullivan
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-02-10 22:09 PST by John Sullivan
Modified: 2009-02-11 06:12 PST (History)
0 users

See Also:


Attachments
Patch to correct negative visit count values found in history read from disk (2.96 KB, patch)
2009-02-10 22:19 PST, John Sullivan
mitz: review-
Details | Formatted Diff | Diff
Revised patch that uses max of 0 instead of 1 for daily & weekly counts (2.88 KB, patch)
2009-02-10 22:33 PST, John Sullivan
mitz: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description John Sullivan 2009-02-10 22:09:30 PST
We've had at least one report of a history item with a huge negative visit count on disk (<rdar://6572300>). This can mess up any algorithms that use the visit count, obviously.

WebCore stores visit count as an int, and has no guard against overflow an int, but it seems highly unlikely that overflow occurred, at least via user interaction with the web.

Data on disk can't be trusted, so we should turn negative values into something sane when reading history from the disk.
Comment 1 John Sullivan 2009-02-10 22:19:56 PST
Created attachment 27554 [details]
Patch to correct negative visit count values found in history read from disk
Comment 2 mitz 2009-02-10 22:22:09 PST
Comment on attachment 27554 [details]
Patch to correct negative visit count values found in history read from disk

There can be a daily visit count sequence of e.g. (1, 0, 1).
Comment 3 John Sullivan 2009-02-10 22:26:59 PST
OK, good to know. I'll update the patch to use a max of 0 rather than 1 for daily/weekly visit counts.
Comment 4 John Sullivan 2009-02-10 22:33:36 PST
Created attachment 27555 [details]
Revised patch that uses max of 0 instead of 1 for daily & weekly counts
Comment 5 mitz 2009-02-10 22:36:08 PST
Comment on attachment 27555 [details]
Revised patch that uses max of 0 instead of 1 for daily & weekly counts

r=me
It would be nicer to use max instead of MAX, since this is a C++ file. You may need to add "using namespace std" in order to be able to do it.
Comment 6 John Sullivan 2009-02-10 22:39:09 PST
Changed MAX() to max() as suggested; committed revision 40851.
Comment 7 Darin Adler 2009-02-11 06:11:41 PST
Comment on attachment 27555 [details]
Revised patch that uses max of 0 instead of 1 for daily & weekly counts

Normally we'd use the max function from <algorithm> instead of the MAX macro, since the macro evaluates its arguments twice.
Comment 8 Darin Adler 2009-02-11 06:12:45 PST
I see now that Dan already asked you to use max, and that you did it!