You are viewing a plain text version of this content. The canonical link for it is here.
Posted to bugs@httpd.apache.org by bu...@apache.org on 2006/07/14 08:44:58 UTC

DO NOT REPLY [Bug 40043] New: - ap_get_client_block discards anything over specified buffer size and return 0 on a subsequent call

DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG�
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=40043>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND�
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=40043

           Summary: ap_get_client_block discards anything over specified
                    buffer size and return 0 on a subsequent call
           Product: Apache httpd-2
           Version: 2.2-HEAD
          Platform: All
        OS/Version: All
            Status: NEW
          Severity: major
          Priority: P2
         Component: Core
        AssignedTo: bugs@httpd.apache.org
        ReportedBy: haiping@plaxo.com


I logged this bug against httpd 2.0.58, the version I have, but reading source 
code of 2.2.0 and 2.2.2, the bug is still there for sure. I didn't check 2.3.

When ap_get_client_block() is called with a buffer size say 48k, and it is 
called again, even if whole brigade has more data (than 48k), the 2nd call 
will return 0 regardless. This is because internally, both ap_get_client_block 
and apr_brigade_flatten, a function called by ap_get_client_block, will simply 
discard the whole brigade after filling up specified buffer in the 1st call. 
This buggy behavior is even pointed out by comments in a .c file, not in the 
header or anywhere in documentation. Several people on the web have 
encountered this same bug one way or another, and there may already be some 
hack or workaround somewhere that I couldn't find, and I'm pretty sure this is 
a known bug, probably not logged though. I'm just copying my own workaround by 
replacing and not calling the original ap_get_client_block(). So one can call 
mine like this sample,

apr_bucket_brigade *m_bb = NULL;
apr_bucket *m_b = NULL;
apr_size_t m_pos = 0;

// this block can be repeated until len_read is 0 or -1
int nBufSize = *piSize;
int len_read;
len_read = ap_get_client_block_ex(m_bb, m_b, m_pos,
                                 (request_rec*) m_pContext, pBuffer, *piSize);

// in case there's an error somewhere, clean up m_bb
if (m_bb) {
  apr_brigade_destroy(m_bb);
}


-- file: http_protocol_ex.h

#include "apr.h"
#include "apr_lib.h"
#include "apr_strings.h"
#include "apr_pools.h"
#include "apr_tables.h"
#include "apr_buckets.h"
#include "apr_errno.h"
#define APR_WANT_MEMFUNC
#define APR_WANT_STRFUNC
#include "apr_want.h"

#include "httpd.h"
#include "http_config.h"
#include "http_core.h"
#include "http_protocol.h"

AP_DECLARE(long) ap_get_client_block_ex(apr_bucket_brigade *&bb,
                                        apr_bucket *&b,
                                        apr_size_t &pos,
                                        request_rec *r, char *buffer,
                                        apr_size_t bufsiz);

APU_DECLARE(apr_status_t) apr_brigade_flatten_ex(apr_bucket_brigade *bb,
                                                 apr_bucket *&b,
                                                 apr_size_t &pos,
                                                 char *c, apr_size_t *len);



-- file http_protocol_ex.c

// where i can fix apache 2.0 bugs
#include "http_protocol_ex.h"

AP_DECLARE(long) ap_get_client_block_ex(apr_bucket_brigade *&bb,
                                        apr_bucket *&b,
                                        apr_size_t &pos,
                                        request_rec *r, char *buffer,
                                        apr_size_t bufsiz)
{
  apr_status_t rv;

  if (r->remaining < 0 || (!r->read_chunked && r->remaining == 0)) {
    return 0;
  }

  if (bb == NULL) {

    bb = apr_brigade_create(r->pool, r->connection->bucket_alloc);
    if (bb == NULL) {
      r->connection->keepalive = AP_CONN_CLOSE;
      return -1;
    }

    rv = ap_get_brigade(r->input_filters, bb, AP_MODE_READBYTES,
                        APR_BLOCK_READ, bufsiz);

    /* We lose the failure code here.  This is why ap_get_client_block should
     * not be used.
     */
    if (rv != APR_SUCCESS) {
      /* if we actually fail here, we want to just return and
       * stop trying to read data from the client.
       */
      r->connection->keepalive = AP_CONN_CLOSE;
      apr_brigade_destroy(bb);
      bb = NULL;
      return -1;
    }

    /* If this fails, it means that a filter is written incorrectly and that
     * it needs to learn how to properly handle APR_BLOCK_READ requests by
     * returning data when requested.
     */
    AP_DEBUG_ASSERT(!APR_BRIGADE_EMPTY(bb));

    b = NULL;
  }

  rv = apr_brigade_flatten_ex(bb, b, pos, buffer, &bufsiz);
  if (rv != APR_SUCCESS) {
    apr_brigade_destroy(bb);
    bb = NULL;
    return -1;
  }

  if (b == APR_BRIGADE_SENTINEL(bb)) {
    /* Check to see if EOS in the brigade.
     *
     * If so, we have to leave a nugget for the *next* ap_get_client_block
     * call to return 0.
     */
    if (APR_BUCKET_IS_EOS(APR_BRIGADE_LAST(bb))) {
      if (r->read_chunked) {
        r->remaining = -1;
      }
      else {
        r->remaining = 0;
      }
    }

    apr_brigade_destroy(bb);
    bb = NULL;
  }

  /* XXX yank me? */
  r->read_length += bufsiz;

  return bufsiz;
}

APU_DECLARE(apr_status_t) apr_brigade_flatten_ex(apr_bucket_brigade *bb,
                                                 apr_bucket *&b,
                                                 apr_size_t &pos,
                                                 char *c, apr_size_t *len)
{
  apr_size_t actual = 0;

  for (b = b ? b : APR_BRIGADE_FIRST(bb); b != APR_BRIGADE_SENTINEL(bb); b = 
APR_BUCKET_NEXT(b)) {
    const char *str;
    apr_size_t str_len;
    apr_status_t status;

    status = apr_bucket_read(b, &str, &str_len, APR_BLOCK_READ);
    if (status != APR_SUCCESS) {
      return status;
    }

    if (pos >= str_len) {
      pos = 0;
      continue;
    }

    // determine the block in bucket to copy to c
    apr_size_t pos0 = pos;
    str += pos; str_len -= pos;    // shift window by "pos" to right
    if (str_len + actual > *len) {
      str_len = *len - actual;     // shorten window by "*len - actual" to left
    }

    // copy the block/window
    memcpy(c, str, str_len);

    c += str_len;
    actual += str_len;

    /* This could probably be actual == *len, but be safe from stray
     * photons. */
    if (actual >= *len) {
      pos = pos0 + str_len; // pos always points to the byte to read next time
      break;
    }
    pos = 0; // get ready to read next bucket
  }

  *len = actual;
  return APR_SUCCESS;
}

-- 
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org