Added checks for integer overflow conditions to deleteData and replaceData.

Added checks for integer overflow conditions to deleteData and replaceData.

Also optimised the multiple calls to length() into a single call at the beginning of both functions.

Finally, as requested, I modified the assertion I was hitting to indicate it has a security implication.

BUG=349898

[enne (OOO), 2014-03-06 21:39:29]

I don't really know this code.  Naively, this lgtm, but maybe there's a better
reviewer.

[jbutler, 2014-03-06 22:28:59]

On 2014/03/06 21:39:29, enne wrote:
> I don't really know this code.  Naively, this lgtm, but maybe there's a better
> reviewer.

Ok, no problem. You were suggested by git-cl, but since the file has no owner I
guess you were chosen at random. If you know of any better reviewers, please let
me know and I'll CC them in.

This is my first set of patches, so I imagine there will probably be changes
required.

[enne (OOO), 2014-03-06 22:33:14]

+eseidel, based on git log of CharacterData.cpp

[eseidel, 2014-03-06 23:41:27]

lgtm

[commit-bot: I haz the power, 2014-03-06 23:42:09]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jbutler@chromium.org/188693007/20001

[commit-bot: I haz the power, 2014-03-07 00:22:46]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-07 00:22:47]

Try jobs failed on following builders: mac_blink_rel

[jbutler, 2014-03-07 11:24:53]

Updated layout tests, and confirmed they pass on my setup.

[jbutler, 2014-03-16 10:08:57]

On 2014/03/07 11:24:53, jbutler wrote:
> Updated layout tests, and confirmed they pass on my setup.

Bump. I've checked over the layout tests, they should pass on the trybots now.

[eseidel, 2014-03-16 20:43:29]

lgtm

[commit-bot: I haz the power, 2014-03-16 20:43:34]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jbutler@chromium.org/188693007/60001

[commit-bot: I haz the power, 2014-03-16 21:15:06]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-16 21:15:07]

Try jobs failed on following builders:
  tryserver.blink on mac_blink_rel

[sof, 2014-03-16 21:17:20]

This looks like it breaks the spec (step 3),

  http://dom.spec.whatwg.org/#concept-cd-replace

i.e., the overflow condition shouldn't throw.

Could you comment on compatibility with others?

[Inactive, 2014-03-17 00:04:32]

https://codereview.chromium.org/188693007/diff/60001/LayoutTests/fast/dom/Ran...
File LayoutTests/fast/dom/Range/deleteData-replaceData-count-overflow.html
(right):

https://codereview.chromium.org/188693007/diff/60001/LayoutTests/fast/dom/Ran...
LayoutTests/fast/dom/Range/deleteData-replaceData-count-overflow.html:28: try {
It would also be nice if this test used js-test.js and shouldThrow() so that we
can check for a particular type of exception. This would also make the test
shorter / simpler.

[jbutler, 2014-03-17 07:34:25]

On 2014/03/17 00:04:32, Chris Dumez wrote:
>
https://codereview.chromium.org/188693007/diff/60001/LayoutTests/fast/dom/Ran...
> File LayoutTests/fast/dom/Range/deleteData-replaceData-count-overflow.html
> (right):
> 
>
https://codereview.chromium.org/188693007/diff/60001/LayoutTests/fast/dom/Ran...
> LayoutTests/fast/dom/Range/deleteData-replaceData-count-overflow.html:28: try
{
> It would also be nice if this test used js-test.js and shouldThrow() so that
we
> can check for a particular type of exception. This would also make the test
> shorter / simpler.

Sorry guys, this is my first time writing tests, and there doesn't seem to be
any good info on this kind of thing.

I'll try again later today and see if I can simplify things.

[sof, 2014-03-19 11:40:18]

https://codereview.chromium.org/188693007/diff/60001/Source/core/dom/Characte...
File Source/core/dom/CharacterData.cpp (right):

https://codereview.chromium.org/188693007/diff/60001/Source/core/dom/Characte...
Source/core/dom/CharacterData.cpp:118: if (count > (dataLength - offset)) {
If not already, you may want to consider handling overflow checking using
Checked<unsigned, OverflowHandler> instead.