Bug 117307

Summary:

[curl] Set the http response status text

Product:

WebKit

Reporter:

Peter Gal <galpeter>

Component:

WebCore Misc.

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

bfulgham, commit-queue

Priority:

Keywords:

Curl

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

Bug Blocks:

117300

Attachments:

Description	Flags
proposed patch	none
fixed patch	bfulgham: review-
updated patch	none

Peter Gal

Reported 2013-06-06 09:41:04 PDT

The ResoureResponse class has a HTTP status text field which is not set with the curl backend.

Attachments
proposed patch (3.09 KB, patch) 2013-06-06 09:53 PDT, Peter Gal	no flags	Details Formatted Diff Diff
fixed patch (1.95 KB, patch) 2013-06-07 04:55 PDT, Peter Gal	bfulgham: review-	Details Formatted Diff Diff
updated patch (2.15 KB, patch) 2013-06-14 08:08 PDT, Peter Gal	no flags	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Peter Gal

Comment 1 2013-06-06 09:53:47 PDT

Created attachment 203945 [details] proposed patch Sadly the curl does not provide a way to extract the http status text so a little workaround is needed.

Peter Gal

Comment 2 2013-06-07 03:49:42 PDT

Comment on attachment 203945 [details] proposed patch I've just found out this patch will result a failing http test. So removing the flags and trying to fix it.

Peter Gal

Comment 3 2013-06-07 04:55:08 PDT

Created attachment 204022 [details] fixed patch

Kenneth Rohde Christiansen

Comment 4 2013-06-08 04:16:53 PDT

Comment on attachment 204022 [details] fixed patch View in context: https://bugs.webkit.org/attachment.cgi?id=204022&action=review > Source/WebCore/platform/network/curl/ResourceHandleManager.cpp:321 > + else if (header.startsWith("HTTP", false)) { what does false mean? add /* ... */ ? > Source/WebCore/platform/network/curl/ResourceHandleManager.cpp:323 > + // This is (probably) the first line of the response. > + // Try to extract the http status text from this. not very trustworthy comment, can you be more specific? Attempt to extract HTTP status...

Peter Gal

Comment 5 2013-06-10 02:20:39 PDT

(In reply to comment #4) > (From update of attachment 204022 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=204022&action=review > > > Source/WebCore/platform/network/curl/ResourceHandleManager.cpp:321 > > + else if (header.startsWith("HTTP", false)) { > > what does false mean? add /* ... */ ? The second argument specifies if the string matching should be case sensitive or not. I've never seen that someone adds the comment for this method. But I can do. > > Source/WebCore/platform/network/curl/ResourceHandleManager.cpp:323 > > + // This is (probably) the first line of the response. > > + // Try to extract the http status text from this. > > not very trustworthy comment, can you be more specific? > > Attempt to extract HTTP status... In case of the curl backend the FOLLOWLOCATION is set to 1, that'll mean that the libcurl will internally follow the redirections. Thus the headerCallback method will be called more than one times with the line starting "HTTP".

Brent Fulgham

Comment 6 2013-06-13 10:02:29 PDT

Comment on attachment 204022 [details] fixed patch Peter: Can you please revise the comments on line 322-323 with the additional detail you added in the bug? Once you do, I'll r+ this. I don't think you need to add a comment to the 'startsWith' method, as it's used all-over-the-place.

Peter Gal

Comment 7 2013-06-13 10:07:19 PDT

(In reply to comment #6) > (From update of attachment 204022 [details]) > Peter: Can you please revise the comments on line 322-323 with the additional detail you added in the bug? Once you do, I'll r+ this. I don't think you need to add a comment to the 'startsWith' method, as it's used all-over-the-place. Okay, will do.

Peter Gal

Comment 8 2013-06-14 08:08:28 PDT

Created attachment 204714 [details] updated patch

Brent Fulgham

Comment 9 2013-06-17 23:07:25 PDT

Comment on attachment 204714 [details] updated patch r=me.

WebKit Commit Bot

Comment 10 2013-06-17 23:10:01 PDT

Comment on attachment 204714 [details] updated patch Clearing flags on attachment: 204714 Committed r151668: <http://trac.webkit.org/changeset/151668>

WebKit Commit Bot

Comment 11 2013-06-17 23:10:03 PDT

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.