[Webkit-unassigned] [Bug 117735] [curl] Improve multipart response handling

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=117735





--- Comment #11 from Peter Gal <galpeter at inf.u-szeged.hu>  2013-09-04 01:31:16 PST ---
(In reply to comment #10)
> (From update of attachment 208710 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=208710&action=review
> 
> > Source/WebCore/platform/network/curl/MultipartHandle.cpp:82
> > +        if (data[pos + i] != m_boundary[i]) {
> 
> how do we know data[pos + i] does not go out of bounds?

Ahh yeah, didn't checked for that. I'll fix that.

> > Source/WebCore/platform/network/curl/MultipartHandle.cpp:135
> > +    char* end = const_cast<char*>(content) + contentLength;
> 
> Cannot this one be const?
> 

It can be, and will be :)

> > Source/WebCore/platform/network/curl/MultipartHandle.h:77
> > +    Vector<char> m_buffer;
> 
> Considering the operations you are doing (append and remove from beginning), a Vector may not be the best data structure. You might want to take a look at StreamBuffer (or Deque)
> 

The StreamBuffer operates with the concept of blocks (which are essentially Vectors) and these blocks are in a Deque. Also it is only possible to access the first block only, so if the boundary string is split between 2 block, to correctly handle that case that'll require more code and/or complexity in the MultipartHandler.

The Deque uses internally the same VectorBuffer as the Vector. Also note that if I would use Deque<char> then when appending characters I would need to iterate over all input chars to call append for each one, in Vector there is already such method so I don't need to write it (I know not a big excuse but still less code to type this way :))

> > Source/WebCore/platform/network/curl/ResourceHandleManager.cpp:406
> > +                d->m_multipartHandle = adoptPtr(new MultipartHandle(job, boundary));
> 
> We usually use static factory functions such as PassOwnPtr<MultipartHandle> MultipartHandle::create() for such cases so that we never deal with raw pointers externally.

Okay, that makes sense.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.