Uploaded image for project: 'mod_python'
  1. mod_python
  2. MODPYTHON-238

req.connection.keepalive should be writable

    Details

    • Type: Improvement
    • Status: In Progress
    • Priority: Minor
    • Resolution: Unresolved
    • Affects Version/s: 3.3.1
    • Fix Version/s: 3.3.x
    • Component/s: core
    • Labels:
      None

      Description

      The attribute req.connection.keepalive should be writable. This would allow handlers to use:

      req.connection.keepalive = apache.AP_CONN_CLOSE

      Doing this before any data is written has the effect of disabling keepalive and also turning off chunked encoding for response content when no content length has been supplied.

        Activity

        Hide
        grahamd Graham Dumpleton added a comment -

        Actually also need to make req.chunked writable. As it has to be set to 0 in combination with setting close for keepalive to disabled chunked encoding for response.

        Show
        grahamd Graham Dumpleton added a comment - Actually also need to make req.chunked writable. As it has to be set to 0 in combination with setting close for keepalive to disabled chunked encoding for response.
        Hide
        milosoftware Mike Looijmans added a comment -

        If there is a separate req.chunked, I'd advise to keep them separated. It's perfectly valid to have
        a chunked stream with a connection: close setting, and someone is bound to use that. For one thing,
        it allows the client to reliably detect whether all the data has really been sent.

        Having a side-effect like setting one disables the other is bound to surprise someone.

        Mike Looijmans
        Philips Natlab / Topic Automation

        Show
        milosoftware Mike Looijmans added a comment - If there is a separate req.chunked, I'd advise to keep them separated. It's perfectly valid to have a chunked stream with a connection: close setting, and someone is bound to use that. For one thing, it allows the client to reliably detect whether all the data has really been sent. Having a side-effect like setting one disables the other is bound to surprise someone. Mike Looijmans Philips Natlab / Topic Automation
        Hide
        grahamd Graham Dumpleton added a comment -

        Setting req.chunked to 0 by itself doesn't actually disable chunking. It is all because of some weird code in modules/http/http_protocol.c which gets called on first call to write() and which starts with:

        if ((r->connection->keepalive != AP_CONN_CLOSE)
        && ((r->status == HTTP_NOT_MODIFIED)

        (r->status == HTTP_NO_CONTENT)
        r->header_only
        apr_table_get(r->headers_out, "Content-Length")
        ap_find_last_token(r->pool,
        apr_table_get(r->headers_out,
        "Transfer-Encoding"),
        "chunked")
        ((r->proto_num >= HTTP_VERSION(1,1))
        && (r->chunked = 1))) /* THIS CODE IS CORRECT, see above. */
        && r->server->keep_alive
        && (r->server->keep_alive_timeout > 0)
        && ((r->server->keep_alive_max == 0)
        (r->server->keep_alive_max > r->connection->keepalives))
        && !ap_status_drops_connection(r->status)
        && !wimpy
        && !ap_find_token(r->pool, conn, "close")
        && (!apr_table_get(r->subprocess_env, "nokeepalive")
        apr_table_get(r->headers_in, "Via"))
        && ((ka_sent = ap_find_token(r->pool, conn, "keep-alive"))
        (r->proto_num >= HTTP_VERSION(1,1)))) {

        Thus, if r->connection->keepalive is not AP_CONN_CLOSE, it will overwrite r->chunked as a side affect of the if condition.

        Thus, only way to turn off chunking is to set connection to close as well as turn chunking to off. Ie.,

        r->connection->keepalive = apache.AP_CONN_CLOSE
        r->chunked = 0

        If you don't care whether chunking is used or not and just want to ensure connection is closed, would instead use:

        r->connection->keepalive = apache.AP_CONN_CLOSE

        or (I think):

        r->subprocess_env['keepalive'] = "0"

        or:

        r->headers_out['Connection'] = 'Close'

        All up, all I am doing is making writable things at C API level that module writer may want to change. How they are used is governed by Apache and not changing that. Frankly, some of this stuff is real black magic.

        Show
        grahamd Graham Dumpleton added a comment - Setting req.chunked to 0 by itself doesn't actually disable chunking. It is all because of some weird code in modules/http/http_protocol.c which gets called on first call to write() and which starts with: if ((r->connection->keepalive != AP_CONN_CLOSE) && ((r->status == HTTP_NOT_MODIFIED) (r->status == HTTP_NO_CONTENT) r->header_only apr_table_get(r->headers_out, "Content-Length") ap_find_last_token(r->pool, apr_table_get(r->headers_out, "Transfer-Encoding"), "chunked") ((r->proto_num >= HTTP_VERSION(1,1)) && (r->chunked = 1))) /* THIS CODE IS CORRECT, see above. */ && r->server->keep_alive && (r->server->keep_alive_timeout > 0) && ((r->server->keep_alive_max == 0) (r->server->keep_alive_max > r->connection->keepalives)) && !ap_status_drops_connection(r->status) && !wimpy && !ap_find_token(r->pool, conn, "close") && (!apr_table_get(r->subprocess_env, "nokeepalive") apr_table_get(r->headers_in, "Via")) && ((ka_sent = ap_find_token(r->pool, conn, "keep-alive")) (r->proto_num >= HTTP_VERSION(1,1)))) { Thus, if r->connection->keepalive is not AP_CONN_CLOSE, it will overwrite r->chunked as a side affect of the if condition. Thus, only way to turn off chunking is to set connection to close as well as turn chunking to off. Ie., r->connection->keepalive = apache.AP_CONN_CLOSE r->chunked = 0 If you don't care whether chunking is used or not and just want to ensure connection is closed, would instead use: r->connection->keepalive = apache.AP_CONN_CLOSE or (I think): r->subprocess_env ['keepalive'] = "0" or: r->headers_out ['Connection'] = 'Close' All up, all I am doing is making writable things at C API level that module writer may want to change. How they are used is governed by Apache and not changing that. Frankly, some of this stuff is real black magic.
        Hide
        grahamd Graham Dumpleton added a comment -

        Fixed in revision 687024 of trunk.

        Show
        grahamd Graham Dumpleton added a comment - Fixed in revision 687024 of trunk.

          People

          • Assignee:
            grahamd Graham Dumpleton
            Reporter:
            grahamd Graham Dumpleton
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:

              Development