2016-06-09 16:07 GMT+02:00 Luca Toscano <toscano.luca@gmail.com>:


2016-06-08 13:42 GMT+02:00 Luca Toscano <toscano.luca@gmail.com>:
[+devs]

2016-06-07 23:02 GMT+02:00 Luca Toscano <toscano.luca@gmail.com>:


2016-06-07 10:55 GMT+02:00 Vacelet, Manuel <manuel.vacelet@enalean.com>:


On Mon, Jun 6, 2016 at 5:32 PM, Vacelet, Manuel <manuel.vacelet@enalean.com> wrote:
dOn Mon, Jun 6, 2016 at 5:00 PM, Vacelet, Manuel <manuel.vacelet@enalean.com> wrote:
On Mon, Jun 6, 2016 at 4:09 PM, Luca Toscano <toscano.luca@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:


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:


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