23889 2009-02-10 22:09:30 -0800 Negative visit counts stored in History.plist aren't corrected 2009-02-11 06:12:45 -0800 1 1 1 Unclassified WebKit History 528+ (Nightly build) Mac OS X 10.5 RESOLVED FIXED P2 Normal --- 1 sullivan sullivan oldest_to_newest 109247 0 sullivan 2009-02-10 22:09:30 -0800 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. 109252 1 27554 sullivan 2009-02-10 22:19:56 -0800 Created attachment 27554 Patch to correct negative visit count values found in history read from disk 109253 2 27554 mitz 2009-02-10 22:22:09 -0800 Comment on attachment 27554 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). 109254 3 sullivan 2009-02-10 22:26:59 -0800 OK, good to know. I'll update the patch to use a max of 0 rather than 1 for daily/weekly visit counts. 109257 4 27555 sullivan 2009-02-10 22:33:36 -0800 Created attachment 27555 Revised patch that uses max of 0 instead of 1 for daily & weekly counts 109258 5 27555 mitz 2009-02-10 22:36:08 -0800 Comment on attachment 27555 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. 109259 6 sullivan 2009-02-10 22:39:09 -0800 Changed MAX() to max() as suggested; committed revision 40851. 109281 7 27555 darin 2009-02-11 06:11:41 -0800 Comment on attachment 27555 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. 109283 8 darin 2009-02-11 06:12:45 -0800 I see now that Dan already asked you to use max, and that you did it! 27554 2009-02-10 22:19:56 -0800 2009-02-10 22:22:09 -0800 Patch to correct negative visit count values found in history read from disk 23889_patch.txt text/plain 3026 sullivan SW5kZXg6IG1hYy9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gbWFjL0NoYW5nZUxvZwkocmV2aXNp b24gNDA4NDgpCisrKyBtYWMvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMjIg QEAKKzIwMDktMDItMTAgIEpvaG4gU3VsbGl2YW4gIDxzdWxsaXZhbkBhcHBsZS5jb20+CisKKyAg ICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisgICAgICAgIAorICAgICAgICA8aHR0 cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTIzODg5PiwgPHJkYXI6Ly9wcm9i bGVtLzY1NzIzMDA+CisgICAgICAgIE5lZ2F0aXZlIHZpc2l0IGNvdW50cyBzdG9yZWQgaW4gSGlz dG9yeS5wbGlzdCBhcmVuJ3QgY29ycmVjdGVkLgorICAgICAgICAKKyAgICAgICAgSXQncyBub3Qg Y2xlYXIgaG93IGEgaHVnZSBuZWdhdGl2ZSB2aXNpdCBjb3VudCBlbmRlZCB1cCBpbiBIaXN0b3J5 LnBsaXN0LCBidXQgd2UgY2FuJ3QKKyAgICAgICAgdHJ1c3QgZGF0YSByZWFkIGZyb20gZGlzayBz byB3ZSBjYW4gYXQgbGVhc3QgcmVzZXQgdGhpcyB0byBzb21ldGhpbmcgc2FuZS4gV2ViQ29yZSBo YXMKKyAgICAgICAgbm8gZ3VhcmQgYWdhaW5zdCBhIHZpc2l0IGNvdW50IG92ZXJmbG93aW5nIGFu IGludCwgYnV0IHRoYXQgc2VlbXMgdmVyeSB1bmxpa2VseSB0byBoYXZlCisgICAgICAgIGNhdXNl ZCB0aGlzLgorCisgICAgICAgICogSGlzdG9yeS9XZWJIaXN0b3J5SXRlbS5tbToKKyAgICAgICAg KC1bV2ViSGlzdG9yeUl0ZW0gaW5pdEZyb21EaWN0aW9uYXJ5UmVwcmVzZW50YXRpb246XSk6Cisg ICAgICAgIElmIGEgbmVnYXRpdmUgdmlzaXQgY291bnQgaXMgaW4gdGhlIGRpY3Rpb25hcnksIHJl cGxhY2UgaXQgd2l0aCAxLiBEbyB0aGUgc2FtZSB0aGluZyB3aXRoIAorICAgICAgICB0aGUgZGFp bHkgYW5kIHdlZWtseSBjb3VudHMsIGV4Y2VwdCByZXBsYWNlIDAgd2l0aCAxIGFsc28gaW4gdGhv c2UgY2FzZXMsIHNpbmNlIDAgaXMgbmV2ZXIKKyAgICAgICAgYSBsZWdpdGltYXRlIHZhbHVlIHRo ZXJlIChpdCBtaWdodCBiZSBhIGxlZ2l0aW1hdGUgdmFsdWUgZm9yIHRoZSByZWd1bGFyIHZpc2l0 IGNvdW50LCBkdWUKKyAgICAgICAgdG8gY2VydGFpbiBzcGVjaWFsIGNhc2VzKS4KKwogMjAwOS0w Mi0xMCAgQW5kZXJzIENhcmxzc29uICA8YW5kZXJzY2FAYXBwbGUuY29tPgogCiAgICAgICAgIFJl dmlld2VkIGJ5IE1hcmsgUm93ZS4KSW5kZXg6IG1hYy9IaXN0b3J5L1dlYkhpc3RvcnlJdGVtLm1t Cj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT0KLS0tIG1hYy9IaXN0b3J5L1dlYkhpc3RvcnlJdGVtLm1tCShyZXZpc2lvbiA0 MDgyOSkKKysrIG1hYy9IaXN0b3J5L1dlYkhpc3RvcnlJdGVtLm1tCSh3b3JraW5nIGNvcHkpCkBA IC0zNTgsNyArMzU4LDE0IEBACiAgICAgICAgIGNvcmUoX3ByaXZhdGUpLT5zZXRPcmlnaW5hbFVS TFN0cmluZyhuZXdVUkxTdHJpbmcpOwogICAgIH0gCiAKLSAgICBjb3JlKF9wcml2YXRlKS0+c2V0 VmlzaXRDb3VudChbZGljdCBfd2Via2l0X2ludEZvcktleTp2aXNpdENvdW50S2V5XSk7CisgICAg aW50IHZpc2l0Q291bnQgPSBbZGljdCBfd2Via2l0X2ludEZvcktleTp2aXNpdENvdW50S2V5XTsK KyAgICAKKyAgICAvLyBDYW4ndCB0cnVzdCBkYXRhIG9uIGRpc2ssIGFuZCB3ZSd2ZSBoYWQgYXQg bGVhc3Qgb25lIHJlcG9ydCBvZiB0aGlzICg8cmRhcjovLzY1NzIzMDA+KS4KKyAgICBpZiAodmlz aXRDb3VudCA8IDApIHsKKyAgICAgICAgTE9HX0VSUk9SKCJ2aXNpdCBjb3VudCBmb3IgaGlzdG9y eSBpdGVtIFwiJUBcIiBpcyBuZWdhdGl2ZSAoJWQpLCB3aWxsIGJlIHJlc2V0IHRvIDEiLCBVUkxT dHJpbmcsIHZpc2l0Q291bnQpOworICAgICAgICB2aXNpdENvdW50ID0gMTsKKyAgICB9CisgICAg Y29yZShfcHJpdmF0ZSktPnNldFZpc2l0Q291bnQodmlzaXRDb3VudCk7CiAKICAgICBpZiAoW2Rp Y3QgX3dlYmtpdF9ib29sRm9yS2V5Omxhc3RWaXNpdFdhc0ZhaWx1cmVLZXldKQogICAgICAgICBj b3JlKF9wcml2YXRlKS0+c2V0TGFzdFZpc2l0V2FzRmFpbHVyZSh0cnVlKTsKQEAgLTM4MiwxMCAr Mzg5LDEyIEBACiAgICAgICAgIFZlY3RvcjxpbnQ+IGNvcmVEYWlseUNvdW50cyhbZGFpbHlDb3Vu dHMgY291bnRdKTsKICAgICAgICAgVmVjdG9yPGludD4gY29yZVdlZWtseUNvdW50cyhbd2Vla2x5 Q291bnRzIGNvdW50XSk7CiAKKyAgICAgICAgLy8gRGFpbHkgYW5kIHdlZWtseSBjb3VudHMgYXJl IG9ubHkgc3RvcmVkIGZvciB2YWx1ZXMgPj0gMS4gSWYgdGhlIHZhbHVlIGlzIDAgb3IgbmVnYXRp dmUsCisgICAgICAgIC8vIGl0IGlzIGFuIGVycm9yIGluIHRoZSBkYXRhLCBzbyByZXNldCBpdCB0 byAxLgogICAgICAgICBmb3IgKHNpemVfdCBpID0gMDsgaSA8IGNvcmVEYWlseUNvdW50cy5zaXpl KCk7ICsraSkKLSAgICAgICAgICAgIGNvcmVEYWlseUNvdW50c1tpXSA9IFtbZGFpbHlDb3VudHMg X3dlYmtpdF9udW1iZXJBdEluZGV4OmldIGludFZhbHVlXTsKKyAgICAgICAgICAgIGNvcmVEYWls eUNvdW50c1tpXSA9IE1BWChbW2RhaWx5Q291bnRzIF93ZWJraXRfbnVtYmVyQXRJbmRleDppXSBp bnRWYWx1ZV0sIDEpOwogICAgICAgICBmb3IgKHNpemVfdCBpID0gMDsgaSA8IGNvcmVXZWVrbHlD b3VudHMuc2l6ZSgpOyArK2kpCi0gICAgICAgICAgICBjb3JlV2Vla2x5Q291bnRzW2ldID0gW1t3 ZWVrbHlDb3VudHMgX3dlYmtpdF9udW1iZXJBdEluZGV4OmldIGludFZhbHVlXTsKKyAgICAgICAg ICAgIGNvcmVXZWVrbHlDb3VudHNbaV0gPSBNQVgoW1t3ZWVrbHlDb3VudHMgX3dlYmtpdF9udW1i ZXJBdEluZGV4OmldIGludFZhbHVlXSwgMSk7CiAgICAgCiAgICAgICAgIGNvcmUoX3ByaXZhdGUp LT5hZG9wdFZpc2l0Q291bnRzKGNvcmVEYWlseUNvdW50cywgY29yZVdlZWtseUNvdW50cyk7CiAg ICAgfQo= 27555 2009-02-10 22:33:36 -0800 2009-02-10 22:36:08 -0800 Revised patch that uses max of 0 instead of 1 for daily & weekly counts 23889_patch_2.txt text/plain 2946 sullivan SW5kZXg6IG1hYy9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gbWFjL0NoYW5nZUxvZwkocmV2aXNp b24gNDA4NTApCisrKyBtYWMvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDUgKzEsMjIg QEAKIDIwMDktMDItMTAgIEpvaG4gU3VsbGl2YW4gIDxzdWxsaXZhbkBhcHBsZS5jb20+CiAKKyAg ICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisgICAgICAgIAorICAgICAgICA8aHR0 cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTIzODg5PiwgPHJkYXI6Ly9wcm9i bGVtLzY1NzIzMDA+CisgICAgICAgIE5lZ2F0aXZlIHZpc2l0IGNvdW50cyBzdG9yZWQgaW4gSGlz dG9yeS5wbGlzdCBhcmVuJ3QgY29ycmVjdGVkLgorICAgICAgICAKKyAgICAgICAgSXQncyBub3Qg Y2xlYXIgaG93IGEgaHVnZSBuZWdhdGl2ZSB2aXNpdCBjb3VudCBlbmRlZCB1cCBpbiBIaXN0b3J5 LnBsaXN0LCBidXQgd2UgY2FuJ3QKKyAgICAgICAgdHJ1c3QgZGF0YSByZWFkIGZyb20gZGlzayBz byB3ZSBjYW4gYXQgbGVhc3QgcmVzZXQgdGhpcyB0byBzb21ldGhpbmcgc2FuZS4gV2ViQ29yZSBo YXMKKyAgICAgICAgbm8gZ3VhcmQgYWdhaW5zdCBhIHZpc2l0IGNvdW50IG92ZXJmbG93aW5nIGFu IGludCwgYnV0IHRoYXQgc2VlbXMgdmVyeSB1bmxpa2VseSB0byBoYXZlCisgICAgICAgIGNhdXNl ZCB0aGlzLgorCisgICAgICAgICogSGlzdG9yeS9XZWJIaXN0b3J5SXRlbS5tbToKKyAgICAgICAg KC1bV2ViSGlzdG9yeUl0ZW0gaW5pdEZyb21EaWN0aW9uYXJ5UmVwcmVzZW50YXRpb246XSk6Cisg ICAgICAgIElmIGEgbmVnYXRpdmUgdmlzaXQgY291bnQgaXMgaW4gdGhlIGRpY3Rpb25hcnksIHJl cGxhY2UgaXQgd2l0aCAxLiBJZiBhIG5lZ2F0aXZlIGRhaWx5IG9yCisgICAgICAgIHdlZWtseSB2 aXNpdCBjb3VudCBpcyBpbiB0aGUgZGljdGlvbmFyeSwgcmVwbGFjZSBpdCB3aXRoIDAuIAorCisy MDA5LTAyLTEwICBKb2huIFN1bGxpdmFuICA8c3VsbGl2YW5AYXBwbGUuY29tPgorCiAgICAgICAg IFJldmlld2VkIGJ5IERhbiBCZXJuc3RlaW4KICAgICAgICAgCiAgICAgICAgIDxodHRwczovL2J1 Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MjM4OTE+CkluZGV4OiBtYWMvSGlzdG9yeS9X ZWJIaXN0b3J5SXRlbS5tbQo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBtYWMvSGlzdG9yeS9XZWJIaXN0b3J5SXRl bS5tbQkocmV2aXNpb24gNDA4NTApCisrKyBtYWMvSGlzdG9yeS9XZWJIaXN0b3J5SXRlbS5tbQko d29ya2luZyBjb3B5KQpAQCAtMzU4LDcgKzM1OCwxNCBAQCBzdGF0aWMgV2ViV2luZG93V2F0Y2hl ciAqX3dpbmRvd1dhdGNoZXIgCiAgICAgICAgIGNvcmUoX3ByaXZhdGUpLT5zZXRPcmlnaW5hbFVS TFN0cmluZyhuZXdVUkxTdHJpbmcpOwogICAgIH0gCiAKLSAgICBjb3JlKF9wcml2YXRlKS0+c2V0 VmlzaXRDb3VudChbZGljdCBfd2Via2l0X2ludEZvcktleTp2aXNpdENvdW50S2V5XSk7CisgICAg aW50IHZpc2l0Q291bnQgPSBbZGljdCBfd2Via2l0X2ludEZvcktleTp2aXNpdENvdW50S2V5XTsK KyAgICAKKyAgICAvLyBDYW4ndCB0cnVzdCBkYXRhIG9uIGRpc2ssIGFuZCB3ZSd2ZSBoYWQgYXQg bGVhc3Qgb25lIHJlcG9ydCBvZiB0aGlzICg8cmRhcjovLzY1NzIzMDA+KS4KKyAgICBpZiAodmlz aXRDb3VudCA8IDApIHsKKyAgICAgICAgTE9HX0VSUk9SKCJ2aXNpdCBjb3VudCBmb3IgaGlzdG9y eSBpdGVtIFwiJUBcIiBpcyBuZWdhdGl2ZSAoJWQpLCB3aWxsIGJlIHJlc2V0IHRvIDEiLCBVUkxT dHJpbmcsIHZpc2l0Q291bnQpOworICAgICAgICB2aXNpdENvdW50ID0gMTsKKyAgICB9CisgICAg Y29yZShfcHJpdmF0ZSktPnNldFZpc2l0Q291bnQodmlzaXRDb3VudCk7CiAKICAgICBpZiAoW2Rp Y3QgX3dlYmtpdF9ib29sRm9yS2V5Omxhc3RWaXNpdFdhc0ZhaWx1cmVLZXldKQogICAgICAgICBj b3JlKF9wcml2YXRlKS0+c2V0TGFzdFZpc2l0V2FzRmFpbHVyZSh0cnVlKTsKQEAgLTM4MiwxMCAr Mzg5LDExIEBAIHN0YXRpYyBXZWJXaW5kb3dXYXRjaGVyICpfd2luZG93V2F0Y2hlciAKICAgICAg ICAgVmVjdG9yPGludD4gY29yZURhaWx5Q291bnRzKFtkYWlseUNvdW50cyBjb3VudF0pOwogICAg ICAgICBWZWN0b3I8aW50PiBjb3JlV2Vla2x5Q291bnRzKFt3ZWVrbHlDb3VudHMgY291bnRdKTsK IAorICAgICAgICAvLyBEYWlseSBhbmQgd2Vla2x5IGNvdW50cyA8IDAgYXJlIGVycm9ycyBpbiB0 aGUgZGF0YSByZWFkIGZyb20gZGlzaywgc28gcmVzZXQgdG8gMC4KICAgICAgICAgZm9yIChzaXpl X3QgaSA9IDA7IGkgPCBjb3JlRGFpbHlDb3VudHMuc2l6ZSgpOyArK2kpCi0gICAgICAgICAgICBj b3JlRGFpbHlDb3VudHNbaV0gPSBbW2RhaWx5Q291bnRzIF93ZWJraXRfbnVtYmVyQXRJbmRleDpp XSBpbnRWYWx1ZV07CisgICAgICAgICAgICBjb3JlRGFpbHlDb3VudHNbaV0gPSBNQVgoW1tkYWls eUNvdW50cyBfd2Via2l0X251bWJlckF0SW5kZXg6aV0gaW50VmFsdWVdLCAwKTsKICAgICAgICAg Zm9yIChzaXplX3QgaSA9IDA7IGkgPCBjb3JlV2Vla2x5Q291bnRzLnNpemUoKTsgKytpKQotICAg ICAgICAgICAgY29yZVdlZWtseUNvdW50c1tpXSA9IFtbd2Vla2x5Q291bnRzIF93ZWJraXRfbnVt YmVyQXRJbmRleDppXSBpbnRWYWx1ZV07CisgICAgICAgICAgICBjb3JlV2Vla2x5Q291bnRzW2ld ID0gTUFYKFtbd2Vla2x5Q291bnRzIF93ZWJraXRfbnVtYmVyQXRJbmRleDppXSBpbnRWYWx1ZV0s IDApOwogICAgIAogICAgICAgICBjb3JlKF9wcml2YXRlKS0+YWRvcHRWaXNpdENvdW50cyhjb3Jl RGFpbHlDb3VudHMsIGNvcmVXZWVrbHlDb3VudHMpOwogICAgIH0K