Bug 65856 – [chromium] FormData and WebHTTPBody should support filesystem URL possibly with arbitrary range

• Component: WebCore Misc.
• Status: RESOLVED LATER

[Kinuko Yasuda]

[chromium] FormData and WebHTTPBody should support filesystem URL possibly with arbitrary range.

Right now we can append File, FileRange or BlobURL to FormData/WebHTTPBody, but it'd be great if it could also support filesystem URL with arbitrary range request.

If we do so probably appendBlob should be renamed to appendURL and also we should have appendURLRange for an arbitrary range support.

[Kinuko Yasuda]

Created attachment 103243 [details]
Patch

[Darin Fisher (:fishd, Google)]

Comment on attachment 103243 [details]
Patch

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

> Source/WebCore/platform/network/FormData.cpp:432
> +        if (fileLength < fileStart)

this seems odd.  did you mean for fileLength to be a length or an end offset?
i would have expected to see either:

  if (fileEnd < fileStart)
      return false;

or:

  if (fileLength < 0)
      return false;

> Source/WebKit/chromium/public/WebHTTPBody.h:89
> +    // Append blob or filesystem URL to the list of elments.

nit: elments -> elements

> Source/WebKit/chromium/src/WebHTTPBody.cpp:100
> +        result.url = element.m_url;

perhaps we should assert that the given URL scheme is blob or filesystem here
to help catch misuse as early as possible.

[Darin Fisher (:fishd, Google)]

Comment on attachment 103243 [details]
Patch

r=me assuming (fileLength < 0)

[Darin Fisher (:fishd, Google)]

Comment on attachment 103243 [details]
Patch

r=me assuming (fileLength < 0)

[Kinuko Yasuda]

Created attachment 103475 [details]
Patch for submit

Uploading the updated patch (reflected comments) for the record / for optional further reviews.

[Kinuko Yasuda]

Created attachment 103481 [details]
Patch for submit (added assertion)

[Kinuko Yasuda]

(In reply to comment #2)
> (From update of attachment 103243 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=103243&action=review
> 
> > Source/WebCore/platform/network/FormData.cpp:432
> > +        if (fileLength < fileStart)
> 
> this seems odd.  did you mean for fileLength to be a length or an end offset?

From how it's used/called fileLength looks to be a length rather than an end offset.  I changed it to "if (fileLength < 0)" as your suggestion.

The part originally comes from the code for FileRange, so adding @jianli to make sure I'm not making a wrong change.

[WebKit Review Bot]

Comment on attachment 103481 [details]
Patch for submit (added assertion)

Attachment 103481 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/9336646

New failing tests:
inspector/network-status-non-http.html

[WebKit Review Bot]

Comment on attachment 103481 [details]
Patch for submit (added assertion)

Attachment 103481 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/9344169