Commit 5641415e authored by Yann Ylavic's avatar Yann Ylavic
Browse files

Makes more sense to join these proposals and comments. 

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x@1680055 13f79535-47bb-0310-9956-ffa450edef68
parent a2371e54

STATUS

+21 −38
Changes for STATUS: 21 added lines, 38 removed lines.
Original line number Diff line number Diff line
@@ -126,38 +126,6 @@ PATCHES PROPOSED TO BACKPORT FROM TRUNK: 
         really cover a very narrow set of parameters with the way it skips over backreferences?

         Also, why a new API vs. just setting the field inline?



   * mod_proxy_ajp: Fix client connection errors handling and logged status

                    when it occurs.  PR 56823.

     trunk patch: http://svn.apache.org/r1643537

                  http://svn.apache.org/r1643543

                  http://svn.apache.org/r1674056

     2.4.x patch: trunk works (module CHANGES, docs/log-message-tags)

                  http://people.apache.org/~ylavic/httpd-2.4.x-mod_proxy_ajp-client_failed.patch

                  (for convenience/readability)

     +1: ylavic

     trawick: Eric and I were working through this and I (at least) ran into this

              particular roadblock: output_failed sometimes means that we couldn't

              write to the client and sometimes means that we couldn't read from

              the client; later, output_fails triggers setting the status to 400.

              HTTP status shouldn't be 400 for a problem writing to the client.

              (Maybe I missed some guarantee that 400 is only for an error reading

              enough request body??)  I think it would be good to separate output_failed

              from input_failed so that the code is more clear and we can more

              easily tell that the status code is appropriate.  (And unfortunately

              I don't volunteer to make the simple changes and test them :( )

     ylavic: I think the confusion comes from the (original) name output_failed itself,

             since it really means client_failed (i.e. "dialog with [client] %pI failed"),

             as opposed to backend_failed (i.e. "dialog with backend %pI failed").

             When an error occurs on any side (connection), the first flag used regarding

             4xx/5xx vs DONE is data_sent (i.e. to the client), otherwise {4xx,5xx} are

             returned according to {output,backend}_failed. So the clarification is

             possibly as simple as renaming output_failed => client_failed (r1674056 added

             above), WDYT?

             (Note that ap_map_http_request_error-v2.patch proposed below, which collides as

             explained in the comment there, shows the full/expected result wrt mod_proxy_ajp

             returned status/error/AP_FILTER_ERROR, and is maybe a more general (though bigger)

             backport that could include this one).



   * core: Add ap_errorlog_provider to make ErrorLog logging modular. This

           backport keeps syslog logging as part of httpd core and only adds

           API to allow other modules to be used for error logging.
@@ -238,12 +206,27 @@ PATCHES PROPOSED TO BACKPORT FROM TRUNK: 
                  http://svn.apache.org/r1674056

     2.4.x patch: http://people.apache.org/~ylavic/httpd-2.4.x-ap_map_http_request_error-v2.patch

     +1: ylavic, minfrin

     ylavic: depends on r1657881 above, and also r1643537+r1643543+r1674056 already

             proposed separately (if the latter proposal is accepted before

             this one, I'll take it easily into account with a v3 of the

             2.4.x patch, but currently the code from r1657897+r1665625

             collides, this patch shows the final result on mod_proxy_ajp

             though)

     ylavic: Depends on r1657881 above.

             Comment from trawick below is about r1643537+r1643543+r1674056 which

             was first proposed separately (and now included in this proposal).

     trawick: Eric and I were working through this and I (at least) ran into this

              particular roadblock: output_failed sometimes means that we couldn't

              write to the client and sometimes means that we couldn't read from

              the client; later, output_fails triggers setting the status to 400.

              HTTP status shouldn't be 400 for a problem writing to the client.

              (Maybe I missed some guarantee that 400 is only for an error reading

              enough request body??)  I think it would be good to separate output_failed

              from input_failed so that the code is more clear and we can more

              easily tell that the status code is appropriate.  (And unfortunately

              I don't volunteer to make the simple changes and test them :( )

     ylavic: I think the confusion comes from the (original) name output_failed itself,

             since it really means client_failed (i.e. "dialog with [client] %pI failed"),

             as opposed to backend_failed (i.e. "dialog with backend %pI failed").

             When an error occurs on any side (connection), the first flag used regarding

             4xx/5xx vs DONE is data_sent (i.e. to the client), otherwise {4xx,5xx} are

             returned according to {output,backend}_failed. So the clarification is

             possibly as simple as renaming output_failed => client_failed (r1674056 added

             above), WDYT?



  *) core: Cleanup the request soon/even if some output filter fails to

     handle the EOR bucket.