You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Ruediger Pluem <rp...@apache.org> on 2009/09/12 00:06:10 UTC
Re: svn commit: r814048 - /httpd/httpd/trunk/modules/http/http_protocol.c
On 09/11/2009 11:55 PM, wrowe@apache.org wrote:
> Author: wrowe
> Date: Fri Sep 11 21:55:46 2009
> New Revision: 814048
>
> URL: http://svn.apache.org/viewvc?rev=814048&view=rev
> Log:
> Catch the somewhat absurd combination of a 417 error where no Expect header
> was present.
>
> Modified:
> httpd/httpd/trunk/modules/http/http_protocol.c
>
> Modified: httpd/httpd/trunk/modules/http/http_protocol.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http/http_protocol.c?rev=814048&r1=814047&r2=814048&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/http/http_protocol.c (original)
> +++ httpd/httpd/trunk/modules/http/http_protocol.c Fri Sep 11 21:55:46 2009
> @@ -1011,16 +1011,19 @@
> "request-header field overlap the current extent\n"
> "of the selected resource.</p>\n");
> case HTTP_EXPECTATION_FAILED:
> - return(apr_pstrcat(p,
> - "<p>The expectation given in the Expect "
> - "request-header"
> - "\nfield could not be met by this server.</p>\n"
> - "<p>The client sent<pre>\n Expect: ",
> - ap_escape_html(r->pool, apr_table_get(r->headers_in, "Expect")),
> - "\n</pre>\n"
> - "but we only allow the 100-continue "
> - "expectation.</p>\n",
> - NULL));
> + s1 = apr_table_get(r->headers_in, "Expect");
> + if (s1)
> + s1 = apr_pstrcat(p,
> + "<p>The expectation given in the Expect request-header\n"
> + "field could not be met by this server.\n"
> + "The client sent<pre>\n Expect: ",
> + ap_escape_html(r->pool, s1), "\n</pre>\n"
> + NULL);
> + else
> + s1 = "<p>No expectation was seen, the Expect request-header \n"
> + "field was not presented by the client.\n";
> + return add_optional_notes(r, s1, "error-notes", "</p>"
> + "<p>Only the 100-continue expectation is supported.</p>\n");
Hm. Does it make sense to squeeze in the contents of the error-notes between s1
and "<p>Only the 100-continue expectation is supported.</p>\n"?
Why not just return apr_pstrcat(p, s1, "<p>Only the 100-continue expectation is supported.</p>\n", NULL)?
Regards
Rüdiger
Re: svn commit: r814048 - /httpd/httpd/trunk/modules/http/http_protocol.c
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
Ruediger Pluem wrote:
>
> Same here. Fixed in r814114.
Thanks for the catch and the note to STATUS, Ruediger.
Bill
Re: svn commit: r814048 - /httpd/httpd/trunk/modules/http/http_protocol.c
Posted by Ruediger Pluem <rp...@apache.org>.
On 09/12/2009 02:02 AM, Graham Leggett wrote:
> Ruediger Pluem wrote:
>
>> On 09/11/2009 11:55 PM, wrowe@apache.org wrote:
>>> Author: wrowe
>>> Date: Fri Sep 11 21:55:46 2009
>>> New Revision: 814048
>>>
>>> URL: http://svn.apache.org/viewvc?rev=814048&view=rev
>>> Log:
>>> Catch the somewhat absurd combination of a 417 error where no Expect header
>>> was present.
>>>
>>> Modified:
>>> httpd/httpd/trunk/modules/http/http_protocol.c
>
> Just a quick ping - currently getting the error below on trunk:
>
> http_protocol.c: In function 'get_canned_error_string':
> http_protocol.c:1021: error: called object '"\012</pre>\012"' is not a
> function
Same here. Fixed in r814114.
Regards
Rüdiger
Re: svn commit: r814048 - /httpd/httpd/trunk/modules/http/http_protocol.c
Posted by Graham Leggett <mi...@sharp.fm>.
Ruediger Pluem wrote:
> On 09/11/2009 11:55 PM, wrowe@apache.org wrote:
>> Author: wrowe
>> Date: Fri Sep 11 21:55:46 2009
>> New Revision: 814048
>>
>> URL: http://svn.apache.org/viewvc?rev=814048&view=rev
>> Log:
>> Catch the somewhat absurd combination of a 417 error where no Expect header
>> was present.
>>
>> Modified:
>> httpd/httpd/trunk/modules/http/http_protocol.c
Just a quick ping - currently getting the error below on trunk:
http_protocol.c: In function 'get_canned_error_string':
http_protocol.c:1021: error: called object '"\012</pre>\012"' is not a
function
I'm up late and I need to be up early, so can't take a look now, anyone
else see this? (MacOSX v10.5.8).
Regards,
Graham
--
Re: svn commit: r814048 - /httpd/httpd/trunk/modules/http/http_protocol.c
Posted by Ruediger Pluem <rp...@apache.org>.
On 09/12/2009 12:09 AM, William A. Rowe, Jr. wrote:
> Ruediger Pluem wrote:
>> On 09/11/2009 11:55 PM, wrowe@apache.org wrote:
>>> Author: wrowe
>>> Date: Fri Sep 11 21:55:46 2009
>>> New Revision: 814048
>>>
>>> URL: http://svn.apache.org/viewvc?rev=814048&view=rev
>>> Log:
>>> Catch the somewhat absurd combination of a 417 error where no Expect header
>>> was present.
>>>
>>> Modified:
>>> httpd/httpd/trunk/modules/http/http_protocol.c
>>>
>>> Modified: httpd/httpd/trunk/modules/http/http_protocol.c
>>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http/http_protocol.c?rev=814048&r1=814047&r2=814048&view=diff
>>> ==============================================================================
>>> --- httpd/httpd/trunk/modules/http/http_protocol.c (original)
>>> +++ httpd/httpd/trunk/modules/http/http_protocol.c Fri Sep 11 21:55:46 2009
>>> @@ -1011,16 +1011,19 @@
>>> "request-header field overlap the current extent\n"
>>> "of the selected resource.</p>\n");
>>> case HTTP_EXPECTATION_FAILED:
>>> - return(apr_pstrcat(p,
>>> - "<p>The expectation given in the Expect "
>>> - "request-header"
>>> - "\nfield could not be met by this server.</p>\n"
>>> - "<p>The client sent<pre>\n Expect: ",
>>> - ap_escape_html(r->pool, apr_table_get(r->headers_in, "Expect")),
>>> - "\n</pre>\n"
>>> - "but we only allow the 100-continue "
>>> - "expectation.</p>\n",
>>> - NULL));
>>> + s1 = apr_table_get(r->headers_in, "Expect");
>>> + if (s1)
>>> + s1 = apr_pstrcat(p,
>>> + "<p>The expectation given in the Expect request-header\n"
>>> + "field could not be met by this server.\n"
>>> + "The client sent<pre>\n Expect: ",
>>> + ap_escape_html(r->pool, s1), "\n</pre>\n"
>>> + NULL);
>>> + else
>>> + s1 = "<p>No expectation was seen, the Expect request-header \n"
>>> + "field was not presented by the client.\n";
>>> + return add_optional_notes(r, s1, "error-notes", "</p>"
>>> + "<p>Only the 100-continue expectation is supported.</p>\n");
>> Hm. Does it make sense to squeeze in the contents of the error-notes between s1
>> and "<p>Only the 100-continue expectation is supported.</p>\n"?
>> Why not just return apr_pstrcat(p, s1, "<p>Only the 100-continue expectation is supported.</p>\n", NULL)?
>
> Tag Balance. Note that the </p>...<p> is one string.
This wasn't my point :-).
So I rephrase: Why not
return apr_pstrcat(p, s1, "</p><p>Only the 100-continue expectation is supported.</p>\n", NULL)?
instead of the add_optional_notes?
Regards
Rüdiger
Re: svn commit: r814048 - /httpd/httpd/trunk/modules/http/http_protocol.c
Posted by Ruediger Pluem <rp...@apache.org>.
On 09/12/2009 12:13 AM, William A. Rowe, Jr. wrote:
> William A. Rowe, Jr. wrote:
>> Ruediger Pluem wrote:
>>> Hm. Does it make sense to squeeze in the contents of the error-notes between s1
>>> and "<p>Only the 100-continue expectation is supported.</p>\n"?
>>> Why not just return apr_pstrcat(p, s1, "<p>Only the 100-continue expectation is supported.</p>\n", NULL)?
>> Tag Balance. Note that the </p>...<p> is one string.
>
> The reason for introducing error-notes is that this error is entirely
> nonsensical until you consider an -external module- which might introduce
> handling of additional Expect cases (and in some case, require one).
>
> That was the reason, the error-notes can expound upon what is or is not
> supported or required.
Ok, understood, but shouldn't the text from the error-notes appear *after*
<p>Only the 100-continue expectation is supported.</p>
Now it is appearing in *between* s1 and <p>Only the 100-continue expectation is supported.</p>
Regards
Rüdiger
Re: svn commit: r814048 - /httpd/httpd/trunk/modules/http/http_protocol.c
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
William A. Rowe, Jr. wrote:
> Ruediger Pluem wrote:
>> Hm. Does it make sense to squeeze in the contents of the error-notes between s1
>> and "<p>Only the 100-continue expectation is supported.</p>\n"?
>> Why not just return apr_pstrcat(p, s1, "<p>Only the 100-continue expectation is supported.</p>\n", NULL)?
>
> Tag Balance. Note that the </p>...<p> is one string.
The reason for introducing error-notes is that this error is entirely
nonsensical until you consider an -external module- which might introduce
handling of additional Expect cases (and in some case, require one).
That was the reason, the error-notes can expound upon what is or is not
supported or required.
Re: svn commit: r814048 - /httpd/httpd/trunk/modules/http/http_protocol.c
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
Ruediger Pluem wrote:
>
> On 09/11/2009 11:55 PM, wrowe@apache.org wrote:
>> Author: wrowe
>> Date: Fri Sep 11 21:55:46 2009
>> New Revision: 814048
>>
>> URL: http://svn.apache.org/viewvc?rev=814048&view=rev
>> Log:
>> Catch the somewhat absurd combination of a 417 error where no Expect header
>> was present.
>>
>> Modified:
>> httpd/httpd/trunk/modules/http/http_protocol.c
>>
>> Modified: httpd/httpd/trunk/modules/http/http_protocol.c
>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http/http_protocol.c?rev=814048&r1=814047&r2=814048&view=diff
>> ==============================================================================
>> --- httpd/httpd/trunk/modules/http/http_protocol.c (original)
>> +++ httpd/httpd/trunk/modules/http/http_protocol.c Fri Sep 11 21:55:46 2009
>> @@ -1011,16 +1011,19 @@
>> "request-header field overlap the current extent\n"
>> "of the selected resource.</p>\n");
>> case HTTP_EXPECTATION_FAILED:
>> - return(apr_pstrcat(p,
>> - "<p>The expectation given in the Expect "
>> - "request-header"
>> - "\nfield could not be met by this server.</p>\n"
>> - "<p>The client sent<pre>\n Expect: ",
>> - ap_escape_html(r->pool, apr_table_get(r->headers_in, "Expect")),
>> - "\n</pre>\n"
>> - "but we only allow the 100-continue "
>> - "expectation.</p>\n",
>> - NULL));
>> + s1 = apr_table_get(r->headers_in, "Expect");
>> + if (s1)
>> + s1 = apr_pstrcat(p,
>> + "<p>The expectation given in the Expect request-header\n"
>> + "field could not be met by this server.\n"
>> + "The client sent<pre>\n Expect: ",
>> + ap_escape_html(r->pool, s1), "\n</pre>\n"
>> + NULL);
>> + else
>> + s1 = "<p>No expectation was seen, the Expect request-header \n"
>> + "field was not presented by the client.\n";
>> + return add_optional_notes(r, s1, "error-notes", "</p>"
>> + "<p>Only the 100-continue expectation is supported.</p>\n");
>
> Hm. Does it make sense to squeeze in the contents of the error-notes between s1
> and "<p>Only the 100-continue expectation is supported.</p>\n"?
> Why not just return apr_pstrcat(p, s1, "<p>Only the 100-continue expectation is supported.</p>\n", NULL)?
Tag Balance. Note that the </p>...<p> is one string.