2016-06-09 16:07 GMT+02:00 Luca Toscano <toscano.l...@gmail.com>:
>
>
> 2016-06-08 13:42 GMT+02:00 Luca Toscano <toscano.l...@gmail.com>:
>
>> [+devs]
>>
>> 2016-06-07 23:02 GMT+02:00 Luca Toscano <toscano.l...@gmail.com>:
>>
>>>
>>>
>>> 2016-06-07 10:55 GMT+02:00 Vacelet, Manuel <manuel.vace...@enalean.com>:
>>>
>>>>
>>>>
>>>> On Mon, Jun 6, 2016 at 5:32 PM, Vacelet, Manuel <
>>>> manuel.vace...@enalean.com> wrote:
>>>>
>>>>> dOn Mon, Jun 6, 2016 at 5:00 PM, Vacelet, Manuel <
>>>>> manuel.vace...@enalean.com> wrote:
>>>>>
>>>>>> On Mon, Jun 6, 2016 at 4:09 PM, Luca Toscano <toscano.l...@gmail.com>
>>>>>> wrote:
>>>>>>
>>>>>>>
>>>>>>> I was able to repro building httpd from 2.4.x branch and following
>>>>>>> your configuration files on github. I am almost sure that somewhere
>>>>>>> httpd
>>>>>>> sets the Last-Modified header translating "foo" to the first Jan 1970
>>>>>>> date.
>>>>>>> I realized though that I didn't recall the real issue, since passing
>>>>>>> value
>>>>>>> not following the RFC can lead to inconsistencies, so I went back and
>>>>>>> checked the correspondence. Quoting:
>>>>>>>
>>>>>>> "Actually I wrote this snippet to highlight the behaviour (the
>>>>>>> original code sent the date in iso8601 instead of rfc1123) because it
>>>>>>> was
>>>>>>> more obvious.
>>>>>>> During my tests (this is extracted from an automated test suite),
>>>>>>> even after having converted dates to rfc1123, I continued to get some
>>>>>>> sparse errors. What I got is that the value I sent was sometimes
>>>>>>> slightly
>>>>>>> modified (a second or 2) depending on the machine load."
>>>>>>>
>>>>>>> So my understanding is that you would like to know why a
>>>>>>> Last-Modified header with a legitimate date/time set by a PHP app gets
>>>>>>> "delayed" by a couple of seconds from httpd, right?
>>>>>>>
>>>>>>
>>>>>> Yes for sure, this is the primary issue.
>>>>>> However, the (undocumented) difference of behavior from one version
>>>>>> to another (2.2 -> 2.4 and more surprisingly from between two 2.4
>>>>>> versions)
>>>>>> is also in question here.
>>>>>> Even more strange, 2.4 built for other distrib doesn't highlight the
>>>>>> behaviour !
>>>>>>
>>>>>>
>>>>>
>>>>> I made another series of test and it seems to be linked to fastcgi.
>>>>>
>>>>> I took the stock apache (2.4.6 plus tons of patches) & php-fpm
>>>>> (5.4.16 + tons of patches) from RHEL7 and I get the exact same behaviour
>>>>> (headers rewritten to EPOCH)
>>>>> However, if I server the very same php script from mod_php (instead of
>>>>> fcgi) it "works" (the headers are not modified).
>>>>>
>>>>>
>>>> For the record, I also have the same behaviour (headers rewritten when
>>>> using php-fpm + fastcgi) on alpine linux 3.4 that ships apache2-2.4.20.
>>>> So AFAICT, it doesn't seem distro specific.
>>>>
>>>> On the root of the problem, from my point of view:
>>>> - the difference between mod_php vs. php-fpm + fcgi is understandable
>>>> (even if not desired and not documented).
>>>> - the fact that fcgi handler parse & rewrite headers seems to lead to
>>>> inconsistencies (I'll try to build a test case for that).
>>>> - however, even if the headers are wrong, I think apache default (use
>>>> EPOCH) is wrong as it leads to very inconsistent behaviour (the resource
>>>> will never expire). I would prefer either:
>>>> -- do not touch the header
>>>> -- raise a warning and discard the header
>>>>
>>>> What do you think ?
>>>>
>>>
>>>
>>> From my tests the following snippet of code should be responsible for
>>> the switch from 'foo' to unix epoch:
>>>
>>> *https://github.com/apache/httpd/blob/2.4.x/server/util_script.c#L663
>>> <https://github.com/apache/httpd/blob/2.4.x/server/util_script.c#L663>*
>>>
>>> The function that contains the code, ap_scan_script_header_err_core_ex,
>>> is wrapped by a lot of other functions eventually called by modules like
>>> mod-proxy-fcgi. A more verbose description of the function in:
>>>
>>> https://github.com/apache/httpd/blob/2.4.x/include/util_script.h#L200
>>>
>>> Not sure what would be the best thing to do, but probably we could
>>> follow up in a official apache bugzilla task?
>>> https://bz.apache.org/bugzilla/enter_bug.cgi?product=Apache%20httpd-2
>>>
>>> Any thoughts by other readers of the email list?
>>>
>>
>> More specifically, the following patch let the "foo" Last-Modified header
>> to be returned instead of unix epoch:
>>
>> --- server/util_script.c (revision 1747375)
>> +++ server/util_script.c (working copy)
>> @@ -665,8 +665,13 @@
>> * pass it on blindly because of restrictions on future values.
>> */
>> else if (!strcasecmp(w, "Last-Modified")) {
>> - ap_update_mtime(r, apr_date_parse_http(l));
>> - ap_set_last_modified(r);
>> + apr_time_t last_modified_date = apr_date_parse_http(l);
>> + if (last_modified_date) {
>> + ap_update_mtime(r, last_modified_date);
>> + ap_set_last_modified(r);
>> + } else {
>> + apr_table_set(r->headers_out, w, l);
>> + }
>> }
>> else if (!strcasecmp(w, "Set-Cookie")) {
>> apr_table_add(cookie_table, w, l);
>>
>> Omitting the "else" branch will force httpd to drop anything that is not
>> a date in Last-Modified (like 'foo'). Of course this patch is only a proof
>> of concept, it is not meant to be the final solution :)
>>
>> Reading https://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html I am not
>> sure what would be the best course of action.
>>
>> I added the httpd-dev mailing list to get some opinion. Steps to repro
>> are contained in https://bugs.centos.org/view.php?id=10940
>>
>>
> More specific:
>
> ap_scan_script_header_err_core_ex in server/utils.c (that should be used
> by mod-proxy-fcgi) checks headers returned from fcgi scripts and translates
> non RFC compliant Last-Modified header values to unix epoch. For example,
> Last-Modified: foo is returned to the client as Last-Modified: Thu, 01 Jan
> 1970 00:00:00 GMT.
>
> What would be the correct behavior in this case? Not returning any
> Last-Modified to the client (and maybe logging an error/warning?),
> returning the non compliant value as it is, returning Last-Modified: now(),
> other?
>
> Any help would really be appreciated :)
>
After a chat in #httpd-dev I am proposing this trunk patch:
Index: server/util_script.c
===================================================================
--- server/util_script.c (revision 1747855)
+++ server/util_script.c (working copy)
@@ -662,11 +662,19 @@
}
/*
* If the script gave us a Last-Modified header, we can't just
- * pass it on blindly because of restrictions on future values.
+ * pass it on blindly because of restrictions on future or invalid
values.
*/
else if (!ap_cstr_casecmp(w, "Last-Modified")) {
- ap_update_mtime(r, apr_date_parse_http(l));
- ap_set_last_modified(r);
+ apr_time_t last_modified_date = apr_date_parse_http(l);
+ if (last_modified_date) {
+ ap_update_mtime(r, apr_date_parse_http(l));
+ ap_set_last_modified(r);
+ }
+ else {
+ if (APLOGrtrace1(r))
+ ap_log_rerror(SCRIPT_LOG_MARK, APLOG_TRACE1, 0, r,
+ "Invalid header value: Last-Modified:
'%s'", l);
+ }
}
else if (!ap_cstr_casecmp(w, "Set-Cookie")) {
apr_table_add(cookie_table, w, l);
Tested also on 2.4.x, it correctly drops (and log) headers like
'Last-Modified: foo'. This would be my first commit outside the
documentation realm so any feedback would be really great. In case nobody
is against this patch I'll submit a trunk commit during the next days.
Thanks!
Luca
