You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Sascha Schumann <sa...@schumann.cx> on 2000/11/03 00:46:21 UTC

Re: cvs commit: apache-2.0/src/main http_core.c http_protocol.c

On 31 Oct 2000 trawick@locus.apache.org wrote:

> trawick     00/10/31 04:30:22
>
>   Modified:    src      CHANGES
>                src/include http_protocol.h
>                src/main http_core.c http_protocol.c
>   Log:
>   Compute the content length (and add appropriate header field) for
>   the response when no content length is available and we can't use
>   chunked encoding.
>
>   This is going to be painful when the response body is huge, so I
>   suspect we'll have additional criteria in the future.

    Why is this filter added to the request output filters by
    default?

    It makes all kind of streaming applications impossible. There
    does not seem to be any option to disable it (from another
    filter).

    - Sascha


Re: cvs commit: apache-2.0/src/main http_core.c http_protocol.c

Posted by David Reid <dr...@jetnet.co.uk>.
Seems to make sense...  Can't be right, sounds too easy :)

> I think the fix is to leave content length filter in there, but if it
> sees a flush bucket prior to EOS it should disable itself and let
> everything flow as it comes.
> 
> A module has to do a flush to let the content flow to the client ASAP
> so we can use that trigger to know what to do.
> 
> Does that sound reasonable?
> 



Re: cvs commit: apache-2.0/src/main http_core.c http_protocol.c

Posted by Jeff Trawick <tr...@bellsouth.net>.
> I think the fix is to leave content length filter in there, but if it
> sees a flush bucket prior to EOS it should disable itself and let
> everything flow as it comes.
> 
> A module has to do a flush to let the content flow to the client ASAP
> so we can use that trigger to know what to do.

I just committed a change which causes the content length filter to
stop collecting content if it gets a flush bucket.  I think this is
right in general and I *hope* that it helps PHP.  To play with the
change, I added a call to ap_rflush() to mod_autoindex.  It did the
right thing as far as I could tell.

-- 
Jeff Trawick | trawick@ibm.net | PGP public key at web site:
     http://www.geocities.com/SiliconValley/Park/9289/
          Born in Roswell... married an alien...

Re: cvs commit: apache-2.0/src/main http_core.c http_protocol.c

Posted by Jeff Trawick <tr...@bellsouth.net>.
Sascha Schumann <sa...@schumann.cx> writes:

> > I think the fix is to leave content length filter in there, but if it
> > sees a flush bucket prior to EOS it should disable itself and let
> > everything flow as it comes.
> >
> > A module has to do a flush to let the content flow to the client ASAP
> > so we can use that trigger to know what to do.
> >
> > Does that sound reasonable?
> 
>     It does, but I'm not sure that adding the overhead of
>     examining each bucket is the best option.

I feel your pain...

>     We have no_cache and no_local_copy in the request structure.
>     How about no_content_length?

Another idea: require a flush bucket, if any, to be the last bucket in
a brigade (often it will be the only bucket)... this would eliminate the
need to walk through the brigade...

I'm not sure which I dislike the least:

. another flag
. the requirement that a flush bucket can appear only at the end
. walking through the entire brigade

I'm leaning towards the requirement that the flush bucket can appear
only at the end...  This would *seem* to be a very natural requirement.
Currently, not all filters have logic to process the flush bucket
specially, but any such filter is liable to break streaming output so
I guess they'll get fixed PDQ anyway.

This issue isn't unique to content-length calculation. Consider
calculating the md5.  As with content length, it is easy to handle
this in a filter.  This too needs to cease and desist for streaming
output.  I guess it can look at a no_content_length flag or such a
flag can be renamed to streaming or some such.

thoughts?

-- 
Jeff Trawick | trawick@ibm.net | PGP public key at web site:
     http://www.geocities.com/SiliconValley/Park/9289/
          Born in Roswell... married an alien...

Re: cvs commit: apache-2.0/src/main http_core.c http_protocol.c

Posted by Sascha Schumann <sa...@schumann.cx>.
> I think the fix is to leave content length filter in there, but if it
> sees a flush bucket prior to EOS it should disable itself and let
> everything flow as it comes.
>
> A module has to do a flush to let the content flow to the client ASAP
> so we can use that trigger to know what to do.
>
> Does that sound reasonable?

    It does, but I'm not sure that adding the overhead of
    examining each bucket is the best option.

    We have no_cache and no_local_copy in the request structure.
    How about no_content_length?

    - Sascha


Re: cvs commit: apache-2.0/src/main http_core.c http_protocol.c

Posted by Jeff Trawick <tr...@bellsouth.net>.
Jeff Trawick <tr...@bellsouth.net> writes:

> Sascha Schumann <sa...@schumann.cx> writes:
> 
> >     I don't see anything particularly which would help me to make
> >     the PHP module work again.
> > 
> >     Maybe our understanding of the term "streaming" differs.
> 
> Yep... you want the content to go the client ASAP and the filter (for
> HTTP 1.0 when there isn't a content length) wants to hold onto the
> content to calculate the length...
> 
> Comment-out the ap_add_filter() for now...  I don't have time to
> code/commit even that much at the moment...  feel free to do so
> yourself. 

I think the fix is to leave content length filter in there, but if it
sees a flush bucket prior to EOS it should disable itself and let
everything flow as it comes.

A module has to do a flush to let the content flow to the client ASAP
so we can use that trigger to know what to do.

Does that sound reasonable?

-- 
Jeff Trawick | trawick@ibm.net | PGP public key at web site:
     http://www.geocities.com/SiliconValley/Park/9289/
          Born in Roswell... married an alien...

Re: cvs commit: apache-2.0/src/main http_core.c http_protocol.c

Posted by Jeff Trawick <tr...@bellsouth.net>.
Sascha Schumann <sa...@schumann.cx> writes:

> On Thu, 2 Nov 2000 rbb@covalent.net wrote:
> 
> > On Fri, 3 Nov 2000, Sascha Schumann wrote:
> >
> > > > This is fine, because if it is possible for the request to be streamed, we
> > > > just remove the filter.  That is done by the first if statement in the
> > > > function.
> > >
> > >     Could you point me to the condition you refer to?
> >
> > AP_CORE_DECLARE_NONSTD(apr_status_t) ap_content_length_filter(ap_filter_t *f,
> 
>     I know which *function*.
> 
> >         if (r->assbackwards
> >             || r->status == HTTP_NOT_MODIFIED
> >             || r->status == HTTP_NO_CONTENT
> >             || r->header_only
> >             || apr_table_get(r->headers_out, "Content-Length")
> >             || r->proto_num == HTTP_VERSION(1,1)
> >             || ap_find_last_token(f->r->pool,
> >                                   apr_table_get(r->headers_out,
> >                                                 "Transfer-Encoding"),
> >                                   "chunked")) {
> 
>     But which *condition* are you referring to?
> 
>     I don't see anything particularly which would help me to make
>     the PHP module work again.
> 
>     Maybe our understanding of the term "streaming" differs.

Yep... you want the content to go the client ASAP and the filter (for
HTTP 1.0 when there isn't a content length) wants to hold onto the
content to calculate the length...

Comment-out the ap_add_filter() for now...  I don't have time to
code/commit even that much at the moment...  feel free to do so
yourself. 

This is troublesome...  content-length can be useful; we just don't
know if we're going to break dynamic content by doing so...

*&^%
-- 
Jeff Trawick | trawick@ibm.net | PGP public key at web site:
     http://www.geocities.com/SiliconValley/Park/9289/
          Born in Roswell... married an alien...

Re: cvs commit: apache-2.0/src/main http_core.c http_protocol.c

Posted by Sascha Schumann <sa...@schumann.cx>.
On Thu, 2 Nov 2000 rbb@covalent.net wrote:

> On Fri, 3 Nov 2000, Sascha Schumann wrote:
>
> > > This is fine, because if it is possible for the request to be streamed, we
> > > just remove the filter.  That is done by the first if statement in the
> > > function.
> >
> >     Could you point me to the condition you refer to?
>
> AP_CORE_DECLARE_NONSTD(apr_status_t) ap_content_length_filter(ap_filter_t *f,

    I know which *function*.

>         if (r->assbackwards
>             || r->status == HTTP_NOT_MODIFIED
>             || r->status == HTTP_NO_CONTENT
>             || r->header_only
>             || apr_table_get(r->headers_out, "Content-Length")
>             || r->proto_num == HTTP_VERSION(1,1)
>             || ap_find_last_token(f->r->pool,
>                                   apr_table_get(r->headers_out,
>                                                 "Transfer-Encoding"),
>                                   "chunked")) {

    But which *condition* are you referring to?

    I don't see anything particularly which would help me to make
    the PHP module work again.

    Maybe our understanding of the term "streaming" differs.

    - Sascha


Re: cvs commit: apache-2.0/src/main http_core.c http_protocol.c

Posted by rb...@covalent.net.
On Fri, 3 Nov 2000, Sascha Schumann wrote:

> > This is fine, because if it is possible for the request to be streamed, we
> > just remove the filter.  That is done by the first if statement in the
> > function.
> 
>     Could you point me to the condition you refer to?

AP_CORE_DECLARE_NONSTD(apr_status_t) ap_content_length_filter(ap_filter_t *f,
                                                              ap_bucket_brigade *b)
{
    request_rec *r = f->r;
    struct content_length_ctx *ctx;
    apr_status_t rv;
    ap_bucket *e;

    ctx = f->ctx;
    if (!ctx) { /* first time through */
        /* We won't compute a content length if one of the following is true:
         * . subrequest
         * . HTTP/0.9
         * . status HTTP_NOT_MODIFIED or HTTP_NO_CONTENT
         * . HEAD
         * . content length already computed
         * . can be chunked
         * . body already chunked
         * Much of this should correspond to checks in ap_set_keepalive().
         */
        if (r->assbackwards 
            || r->status == HTTP_NOT_MODIFIED 
            || r->status == HTTP_NO_CONTENT
            || r->header_only
            || apr_table_get(r->headers_out, "Content-Length")
            || r->proto_num == HTTP_VERSION(1,1)
            || ap_find_last_token(f->r->pool,
                                  apr_table_get(r->headers_out,
                                                "Transfer-Encoding"),
                                  "chunked")) {
            ap_remove_output_filter(f);
            return ap_pass_brigade(f->next, b);
        }

        f->ctx = ctx = apr_pcalloc(r->pool, sizeof(struct content_length_ctx));
    }

_______________________________________________________________________________
Ryan Bloom                        	rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------


Re: cvs commit: apache-2.0/src/main http_core.c http_protocol.c

Posted by Sascha Schumann <sa...@schumann.cx>.
> This is fine, because if it is possible for the request to be streamed, we
> just remove the filter.  That is done by the first if statement in the
> function.

    Could you point me to the condition you refer to?

    - Sascha


Re: cvs commit: apache-2.0/src/main http_core.c http_protocol.c

Posted by rb...@covalent.net.
>     Why is this filter added to the request output filters by
>     default?
> 
>     It makes all kind of streaming applications impossible. There
>     does not seem to be any option to disable it (from another
>     filter).

This is fine, because if it is possible for the request to be streamed, we
just remove the filter.  That is done by the first if statement in the
function.

Ryan
_______________________________________________________________________________
Ryan Bloom                        	rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------