Bug 155739 – [Win] SharedBuffer::copy() can cause crash

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Brent Fulgham]

As reported by Dongseong Hwang back in 2012:
After SharedBuffer::copy(), SharedBuffer::append() often causes segmentation fault, because copy() calls clone->m_buffer.append(m_segments[i], segmentSize) even if
'i' is the last index. The data size of m_segments.last() is often less than segmentSize. So, in the cloned instance m_size < (m_buffer.size() + SUM(m_segments[i].size())).
This patch appends the exact size of the last segment instead of segmentSize.

We don't encounter this on iOS/OS X because the segments always arrive in full segments. However, Windows, and perhaps other ports, may encounter this problem.

This failure is revealed by the TestWebKitAPI "SharedBufferTest::copy())" test case.

[Brent Fulgham]

Created attachment 274631 [details]
Correct Windows test failure.

[Brent Fulgham]

Created attachment 274632 [details]
Patch

[Ryosuke Niwa]

Comment on attachment 274632 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=274632&action=review

> Source/WebCore/platform/SharedBuffer.cpp:272
> +        unsigned positionInSegment = m_size - m_buffer->data.size() - lastIndex * segmentSize;

I'd call this variable sizeOfLastSegment instead.

[Brent Fulgham]

Martin: FYI in case this affects Gtk+.

[Brent Fulgham]

Comment on attachment 274632 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=274632&action=review

>> Source/WebCore/platform/SharedBuffer.cpp:272
>> +        unsigned positionInSegment = m_size - m_buffer->data.size() - lastIndex * segmentSize;
> 
> I'd call this variable sizeOfLastSegment instead.

Will do!

[Brent Fulgham]

Committed in r198530 <http://trac.webkit.org/changeset/198530>.