41913 2010-07-08 15:36:20 -0700 XPath substring function does not correctly handle non-positive values for the position argument 2010-07-12 03:44:13 -0700 1 1 1 Unclassified WebKit XML 528+ (Nightly build) All All RESOLVED FIXED P2 Normal --- 1 steveblock webkit-unassigned ap benm steveblock oldest_to_newest 248593 0 steveblock 2010-07-08 15:36:20 -0700 In the XPath substring function, if a negative value is supplied for the position argument and a length argument is present, we reset the position argument to the first character of the string and adjust the length argument such that the final character in the substring is the same as it would have been if the length were counted from the original negative position value. We then return the substring using the new position and length. However, in the case where no length argument is present, we call String::substring() with the negative position value. Since String::substring() takes two unsigned int arguments, the position value gets wrapped to a large integer and we likely return the empty string because the wrapped position value exceeds the string length. This seems incorrect and is inconsistent with the above case. See Bug 41862 for some background. 249066 1 ap 2010-07-09 10:02:35 -0700 Yes, substring("12345", -1) should return "12345". This is not only about negative numbers -substring("12345", 0) is also wrong for the same reason. 249070 2 ap 2010-07-09 10:06:16 -0700 See <http://www.w3.org/TR/xpath/#function-substring>. 249071 3 steveblock 2010-07-09 10:07:02 -0700 > Yes, substring("12345", -1) should return "12345". This is not only about > negative numbers -substring("12345", 0) is also wrong for the same reason. Yes, I meant non-positive numbers. Patch coming soon. 249085 4 61054 steveblock 2010-07-09 10:17:33 -0700 Created attachment 61054 Patch 249110 5 61054 ap 2010-07-09 10:55:51 -0700 Comment on attachment 61054 Patch > + we reset the position to 1. This matches the current behaviour when a length > + argument is supplied. This fix is not really about consistency - there is a rigorous spec, and we now match it. Does the new test pass in Firefox? r=me 249732 6 steveblock 2010-07-12 02:51:20 -0700 > This fix is not really about consistency - there is a rigorous spec, and we > now match it. OK, have updated the bug title and will land with an updated description > Does the new test pass in Firefox? Yes 249745 7 steveblock 2010-07-12 03:44:13 -0700 Committed r63066: <http://trac.webkit.org/changeset/63066> 61054 2010-07-09 10:17:33 -0700 2010-07-09 10:55:51 -0700 Patch bug-41913-20100709181731.patch text/plain 4669 steveblock SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiA2Mjk2MSkKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMjAgQEAKKzIwMTAtMDctMDkgIFN0ZXZlIEJsb2NrICA8c3RldmVibG9ja0Bnb29n bGUuY29tPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAg IFhQYXRoIHN1YnN0cmluZyBmdW5jdGlvbiBoYXMgaW5jb25zaXN0ZW50IGhhbmRsaW5nIG9mIG5v bi1wb3NpdGl2ZSB2YWx1ZXMgZm9yIHRoZSBwb3NpdGlvbiBhcmd1bWVudAorICAgICAgICBodHRw czovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9NDE5MTMKKworICAgICAgICBUaGlz IHBhdGNoIGNoYW5nZXMgdGhlIGJlaGF2aW9yIG9mIHRoZSBYUGF0aCBldmFsdWF0ZSBmdW5jdGlv biB3aGVuIGEgbm9uLXBvc2l0aXZlCisgICAgICAgIHBvc2l0aW9uIGFyZ3VtZW50IGlzIHN1cHBs aWVkIGFuZCBubyBsZW5ndGggYXJndW1lbnQgaXMgc3VwcGxpZWQuIEluIHRoaXMgY2FzZSwKKyAg ICAgICAgd2UgcmVzZXQgdGhlIHBvc2l0aW9uIHRvIDEuIFRoaXMgbWF0Y2hlcyB0aGUgY3VycmVu dCBiZWhhdmlvdXIgd2hlbiBhIGxlbmd0aAorICAgICAgICBhcmd1bWVudCBpcyBzdXBwbGllZC4K KworICAgICAgICBUZXN0OiBmYXN0L3hwYXRoL3N1YnN0cmluZy1ub24tcG9zaXRpdmUtcG9zdGlv bi5odG1sCisKKyAgICAgICAgKiB4bWwvWFBhdGhGdW5jdGlvbnMuY3BwOgorICAgICAgICAoV2Vi Q29yZTo6WFBhdGg6OkZ1blN1YnN0cmluZzo6ZXZhbHVhdGUpOgorCiAyMDEwLTA3LTA5ICBLZW5u ZXRoIFJ1c3NlbGwgIDxrYnJAZ29vZ2xlLmNvbT4KIAogICAgICAgICBSZXZpZXdlZCBieSBEaW1p dHJpIEdsYXprb3YuCkluZGV4OiBXZWJDb3JlL3htbC9YUGF0aEZ1bmN0aW9ucy5jcHAKPT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PQotLS0gV2ViQ29yZS94bWwvWFBhdGhGdW5jdGlvbnMuY3BwCShyZXZpc2lvbiA2Mjk1MykK KysrIFdlYkNvcmUveG1sL1hQYXRoRnVuY3Rpb25zLmNwcAkod29ya2luZyBjb3B5KQpAQCAtNTE2 LDExICs1MTYsMTMgQEAgVmFsdWUgRnVuU3Vic3RyaW5nOjpldmFsdWF0ZSgpIGNvbnN0CiAgICAg aWYgKHBvcyA+IGxvbmcocy5sZW5ndGgoKSkpIAogICAgICAgICByZXR1cm4gIiI7CiAKLSAgICBp ZiAoaGF2ZUxlbmd0aCAmJiBwb3MgPCAxKSB7Ci0gICAgICAgIGxlbiAtPSAxIC0gcG9zOworICAg IGlmIChwb3MgPCAxKSB7CisgICAgICAgIGlmIChoYXZlTGVuZ3RoKSB7CisgICAgICAgICAgICBs ZW4gLT0gMSAtIHBvczsKKyAgICAgICAgICAgIGlmIChsZW4gPCAxKQorICAgICAgICAgICAgICAg IHJldHVybiAiIjsKKyAgICAgICAgfQogICAgICAgICBwb3MgPSAxOwotICAgICAgICBpZiAobGVu IDwgMSkKLSAgICAgICAgICAgIHJldHVybiAiIjsKICAgICB9CiAKICAgICByZXR1cm4gcy5zdWJz dHJpbmcocG9zIC0gMSwgbGVuKTsKSW5kZXg6IExheW91dFRlc3RzL0NoYW5nZUxvZwo9PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09Ci0tLSBMYXlvdXRUZXN0cy9DaGFuZ2VMb2cJKHJldmlzaW9uIDYyOTYxKQorKysgTGF5b3V0 VGVzdHMvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMTMgQEAKKzIwMTAtMDct MDkgIFN0ZXZlIEJsb2NrICA8c3RldmVibG9ja0Bnb29nbGUuY29tPgorCisgICAgICAgIFJldmll d2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIFhQYXRoIHN1YnN0cmluZyBmdW5jdGlv biBoYXMgaW5jb25zaXN0ZW50IGhhbmRsaW5nIG9mIG5vbi1wb3NpdGl2ZSB2YWx1ZXMgZm9yIHRo ZSBwb3NpdGlvbiBhcmd1bWVudAorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93 X2J1Zy5jZ2k/aWQ9NDE5MTMKKworICAgICAgICAqIGZhc3QveHBhdGgvc3Vic3RyaW5nLW5vbi1w b3NpdGl2ZS1wb3N0aW9uLWV4cGVjdGVkLnR4dDogQWRkZWQuCisgICAgICAgICogZmFzdC94cGF0 aC9zdWJzdHJpbmctbm9uLXBvc2l0aXZlLXBvc3Rpb24uaHRtbDogQWRkZWQuCisKIDIwMTAtMDct MDkgIEtlbm5ldGggUnVzc2VsbCAgPGtickBnb29nbGUuY29tPgogCiAgICAgICAgIFJldmlld2Vk IGJ5IERpbWl0cmkgR2xhemtvdi4KSW5kZXg6IExheW91dFRlc3RzL2Zhc3QveHBhdGgvc3Vic3Ry aW5nLW5vbi1wb3NpdGl2ZS1wb3N0aW9uLWV4cGVjdGVkLnR4dAo9PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBMYXlv dXRUZXN0cy9mYXN0L3hwYXRoL3N1YnN0cmluZy1ub24tcG9zaXRpdmUtcG9zdGlvbi1leHBlY3Rl ZC50eHQJKHJldmlzaW9uIDApCisrKyBMYXlvdXRUZXN0cy9mYXN0L3hwYXRoL3N1YnN0cmluZy1u b24tcG9zaXRpdmUtcG9zdGlvbi1leHBlY3RlZC50eHQJKHJldmlzaW9uIDApCkBAIC0wLDAgKzEs MTAgQEAKK1Rlc3QgZm9yIGJ1ZyA0MTkxMzogWFBhdGggc3Vic3RyaW5nIGZ1bmN0aW9uIGhhcyBp bmNvbnNpc3RlbnQgaGFuZGxpbmcgb2YgbmVnYXRpdmUgdmFsdWVzIGZvciB0aGUgcG9zaXRpb24g YXJndW1lbnQKKworUEFTUyBkb2N1bWVudC5ldmFsdWF0ZSgic3Vic3RyaW5nKCdhYmNkZScsIDAp IiwgZG9jdW1lbnQsIG51bGwsIFhQYXRoUmVzdWx0LlNUUklOR19UWVBFLCBudWxsKS5zdHJpbmdW YWx1ZSBpcyAnYWJjZGUnCitQQVNTIGRvY3VtZW50LmV2YWx1YXRlKCJzdWJzdHJpbmcoJ2FiY2Rl JywgLTIpIiwgZG9jdW1lbnQsIG51bGwsIFhQYXRoUmVzdWx0LlNUUklOR19UWVBFLCBudWxsKS5z dHJpbmdWYWx1ZSBpcyAnYWJjZGUnCitQQVNTIGRvY3VtZW50LmV2YWx1YXRlKCJzdWJzdHJpbmco J2FiY2RlJywgMCwgNSkiLCBkb2N1bWVudCwgbnVsbCwgWFBhdGhSZXN1bHQuU1RSSU5HX1RZUEUs IG51bGwpLnN0cmluZ1ZhbHVlIGlzICdhYmNkJworUEFTUyBkb2N1bWVudC5ldmFsdWF0ZSgic3Vi c3RyaW5nKCdhYmNkZScsIC0yLCA1KSIsIGRvY3VtZW50LCBudWxsLCBYUGF0aFJlc3VsdC5TVFJJ TkdfVFlQRSwgbnVsbCkuc3RyaW5nVmFsdWUgaXMgJ2FiJworUEFTUyBzdWNjZXNzZnVsbHlQYXJz ZWQgaXMgdHJ1ZQorCitURVNUIENPTVBMRVRFCisKSW5kZXg6IExheW91dFRlc3RzL2Zhc3QveHBh dGgvc3Vic3RyaW5nLW5vbi1wb3NpdGl2ZS1wb3N0aW9uLmh0bWwKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gTGF5 b3V0VGVzdHMvZmFzdC94cGF0aC9zdWJzdHJpbmctbm9uLXBvc2l0aXZlLXBvc3Rpb24uaHRtbAko cmV2aXNpb24gMCkKKysrIExheW91dFRlc3RzL2Zhc3QveHBhdGgvc3Vic3RyaW5nLW5vbi1wb3Np dGl2ZS1wb3N0aW9uLmh0bWwJKHJldmlzaW9uIDApCkBAIC0wLDAgKzEsMjMgQEAKKzwhRE9DVFlQ RSBIVE1MIFBVQkxJQyAiLS8vSUVURi8vRFREIEhUTUwvL0VOIj4KKzxodG1sPgorPGhlYWQ+Cis8 bGluayByZWw9InN0eWxlc2hlZXQiIGhyZWY9Ii4uL2pzL3Jlc291cmNlcy9qcy10ZXN0LXN0eWxl LmNzcyI+Cis8c2NyaXB0IHNyYz0iLi4vanMvcmVzb3VyY2VzL2pzLXRlc3QtcHJlLmpzIj48L3Nj cmlwdD4KKzwvaGVhZD4KKzxib2R5PgorPHA+VGVzdCBmb3IgPGEgaHJlZj0iaHR0cDovL2J1Z3Mu d2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9NDE5MTMiPmJ1ZyA0MTkxMzwvYT46CitYUGF0aCBz dWJzdHJpbmcgZnVuY3Rpb24gaGFzIGluY29uc2lzdGVudCBoYW5kbGluZyBvZiBuZWdhdGl2ZSB2 YWx1ZXMgZm9yIHRoZSBwb3NpdGlvbiBhcmd1bWVudDwvcD4KKzxkaXYgaWQ9ImNvbnNvbGUiPjwv ZGl2PgorCis8c2NyaXB0PgorICAgIHNob3VsZEJlKCJkb2N1bWVudC5ldmFsdWF0ZShcInN1YnN0 cmluZygnYWJjZGUnLCAwKVwiLCBkb2N1bWVudCwgbnVsbCwgWFBhdGhSZXN1bHQuU1RSSU5HX1RZ UEUsIG51bGwpLnN0cmluZ1ZhbHVlIiwgIidhYmNkZSciKTsKKyAgICBzaG91bGRCZSgiZG9jdW1l bnQuZXZhbHVhdGUoXCJzdWJzdHJpbmcoJ2FiY2RlJywgLTIpXCIsIGRvY3VtZW50LCBudWxsLCBY UGF0aFJlc3VsdC5TVFJJTkdfVFlQRSwgbnVsbCkuc3RyaW5nVmFsdWUiLCAiJ2FiY2RlJyIpOwor ICAgIHNob3VsZEJlKCJkb2N1bWVudC5ldmFsdWF0ZShcInN1YnN0cmluZygnYWJjZGUnLCAwLCA1 KVwiLCBkb2N1bWVudCwgbnVsbCwgWFBhdGhSZXN1bHQuU1RSSU5HX1RZUEUsIG51bGwpLnN0cmlu Z1ZhbHVlIiwgIidhYmNkJyIpOworICAgIHNob3VsZEJlKCJkb2N1bWVudC5ldmFsdWF0ZShcInN1 YnN0cmluZygnYWJjZGUnLCAtMiwgNSlcIiwgZG9jdW1lbnQsIG51bGwsIFhQYXRoUmVzdWx0LlNU UklOR19UWVBFLCBudWxsKS5zdHJpbmdWYWx1ZSIsICInYWInIik7CisKKyAgICB2YXIgc3VjY2Vz c2Z1bGx5UGFyc2VkID0gdHJ1ZTsKKworPC9zY3JpcHQ+Cis8c2NyaXB0IHNyYz0iLi4vanMvcmVz b3VyY2VzL2pzLXRlc3QtcG9zdC5qcyI+PC9zY3JpcHQ+Cis8L2JvZHk+Cis8L2h0bWw+Cg==