Re: squid3 comments

From: Tsantilas Christos <chtsanti@dont-contact.us>
Date: Wed, 28 Feb 2007 20:48:35 +0200

Hi Alex,

Alex Rousskov wrote:
>
> I agree that many Squid3 parts should be fixed, polished, or thrown
> away. However, I think that we should focus on making Squid3 stable
> first, and the performance/polishing work you are discussing should be
> done for v3.1. ........................
>

I am not talking about big changes. I am talking about small changes
which can be done while reading the code.

As an example of such changes I am attaching the rewritten
parseHttpRequest, prepareTransparentURL and prepareAcceleratedURL

A second example: again In parseHttpRequest we have the HttpParser
struct which we are using it to parse the first line of request. The
HttpRequest::parseFirstLine method (of HttpMsg derived HttpRequest
class) does more or less the same. Maybe they can merged.

A third example: in HttpStateData::processReplyHeader int http.cc file.
I am seeing the line:
  const bool parsed = newrep->parse(readBuf, eof, &error);

and some lines after I am seeing:
  header_bytes_read = headersEnd(readBuf->content(),
readBuf->contentSize());

What the hell parsing we did in previous line if we did not keep the end
of headers? The easier we can do is to make HttpReply::parse to return
the size of headers on success or zero else. Or just keep in an
internall variable of HttpReply class the size of headers and then get
them with a call like newrep->headersSize()

I think such changes are small and can make squid3 to be a little faster
and can simplify in some cases the code, which will help the debuging.
I am not having a lot of time too but I can find some hours here or
there to make such changes, if needed.

Alex Rousskov wrote:
> ......... There are plenty of users who can use Squid3 that is
> stable but not very fast.
>

This is true, in most setups squid is not needed to be (very) fast. But
in the other hand I am seeing that sometimes it is very difficult to
explain to some people that they do not need a server with 2 or more
fast cpus, expensive hardware raids and fast scsi disks, to make a
file-server for only 10-20 clients ....

Regards,
    Christos

int
internalCheckn(const char *urlpath,int size)
{
    if(size<16)
        return 0;
    return (0 == strncmp(urlpath, "/squid-internal-", 16));
}

char *memstr(char *s,char *pattern,int s_len){
    int i,k;
    int p_len=strlen(pattern);
    for(i=0;i<=s_len-p_len;i++){
      k=0;
      while(k<p_len){
          if(s[i+k] != pattern[k])
             break;
          k++;
      }
      if(k==p_len)
           return s+i;
    }
    return NULL;
}

static void
prepareAcceleratedURL(ConnStateData::Pointer & conn, ClientHttpRequest *http, char *url, int url_size, const char *req_hdr)
{
    int vhost = conn->port->vhost;
    int vport = conn->port->vport;
    int uri_sz = 0, tmpsz = 0;
    char *host,*s;

    http->flags.accel = 1;

    /* BUG: Squid cannot deal with '*' URLs (RFC2616 5.1.2) */

    if (url_size >= 15 && strncasecmp(url, "cache_object://", 15) == 0)
        return; /* already in good shape */

    if (*url != '/') {
        if (conn->port->vhost)
            return; /* already in good shape */

        /* else we need to ignore the host name */
        s = NULL;
        s = memstr(url, "//", u_size);
        if(s)
            u_size -= ((s-url) + 2);
        url = s + 2;

#if SHOULD_REJECT_UNKNOWN_URLS

        if (!url)
            return parseHttpRequestAbort(conn, "error:invalid-request");

#endif

        if (url){
            s=memchr(url,'/',u_size);
            if(s)
                u_size -= s-url;
            url=s;
        }

        if (!url){
            url = (char *) "/";
            url_size = 1;
        }
    }

    if (internalCheckn(url,url_size)) {
        /* prepend our name & port */
        s = (char *)xcalloc(url_size+1,1);
        memcpy(s,url,url_size);
        s[url_size]='\0';
        http->uri = xstrdup(internalLocalUri(NULL, s));
        xfree(s);
        return;
    }

    if (vhost && (host = mime_get_header(req_hdr, "Host")) != NULL) {
        uri_sz = url_size + 32 + Config.appendDomainLen +
                     strlen(host);
        http->uri = (char *)xcalloc(uri_sz, 1);
        tmpsz = snprintf(http->uri, uri_sz, "%s://%s",
                 conn->port->protocol, host);
    } else if (conn->port->defaultsite) {
        uri_sz = url_size + 32 + Config.appendDomainLen +
                     strlen(conn->port->defaultsite);
        http->uri = (char *)xcalloc(uri_sz, 1);
        tmpsz = snprintf(http->uri, uri_sz, "%s://%s",
                 conn->port->protocol, conn->port->defaultsite);
    } else if (vport == -1) {
        /* Put the local socket IP address as the hostname. */
        uri_sz = url_size + 32 + Config.appendDomainLen;
        http->uri = (char *)xcalloc(uri_sz, 1);
        tmpsz = snprintf(http->uri, uri_sz, "%s://%s:%d",
                 http->getConn()->port->protocol,
                 inet_ntoa(http->getConn()->me.sin_addr),
                 ntohs(http->getConn()->me.sin_port));
    } else if (vport > 0) {
        /* Put the local socket IP address as the hostname, but static port */
        uri_sz = url_size + 32 + Config.appendDomainLen;
        http->uri = (char *)xcalloc(uri_sz, 1);
        tmpsz = snprintf(http->uri, uri_sz, "%s://%s:%d",
                 http->getConn()->port->protocol,
                 inet_ntoa(http->getConn()->me.sin_addr),
                 vport);
    }
    else
         return;

    /*OK Append the url at the end of uri and we are OK....*/
    uri_sz -= tmpsz;
    url_size = XMIN(uri_sz - 1, url_size);
    memcpy(http->uri+tmpsz, url , url_size);
    http->uri[tmpsz+url_size] = '\0';
    debug(33, 5) ("ACCEL VHOST REWRITE: '%s'\n", http->uri);
}

static void
prepareTransparentURL(ConnStateData::Pointer & conn, ClientHttpRequest *http, char *url, int url_size, const char *req_hdr)
{
    char *host;
    int uri_sz = 0, tmpsz = 0;
    http->flags.transparent = 1;

    if (*url != '/')
        return; /* already in good shape */

    /* BUG: Squid cannot deal with '*' URLs (RFC2616 5.1.2) */
    if ((host = mime_get_header(req_hdr, "Host")) != NULL) {
        uri_sz = url_size + 32 + Config.appendDomainLen +
             strlen(host);
        http->uri = (char *)xcalloc(uri_sz, 1);
        tmpsz = snprintf(http->uri, uri_sz, "%s://%s",
                             conn->port->protocol, host);
    } else {
        /* Put the local socket IP address as the hostname. */
        uri_sz = url_size + 32 + Config.appendDomainLen;
        http->uri = (char *)xcalloc(uri_sz, 1);
        tmpsz = snprintf(http->uri, uri_sz, "%s://%s:%d",
                            http->getConn()->port->protocol,
                            inet_ntoa(http->getConn()->me.sin_addr),
                            ntohs(http->getConn()->me.sin_port));
    }
    uri_sz -= tmpsz;
    url_size = XMIN(uri_sz - 1, url_size);
    memcpy(http->uri+tmpsz, url , url_size);
    http->uri[tmpsz+url_size] = '\0';
    debug(33, 5) ("TRANSPARENT HOST REWRITE: '%s'\n", http->uri);
}

/*
 * parseHttpRequest()
 *
 * Returns
 * NULL on incomplete requests
 * a ClientSocketContext structure on success or failure.
 * Sets result->flags.parsed_ok to 0 if failed to parse the request.
 * Sets result->flags.parsed_ok to 1 if we have a good request.
 */
static ClientSocketContext *
parseHttpRequest(ConnStateData::Pointer & conn, HttpParser *hp, method_t * method_p, HttpVersion *http_ver)
{
    char *url = NULL;
    int url_size = 0;
    char *req_hdr = NULL;
    char *end;
    size_t req_sz;
    ClientHttpRequest *http;
    ClientSocketContext *result;
    StoreIOBuffer tempBuffer;
    int r;

    /* pre-set these values to make aborting simpler */
    *method_p = METHOD_NONE;

    /* Attempt to parse the first line; this'll define the method, url, version and header begin */
    r = HttpParserParseReqLine(hp);
    if (r == 0) {
        debug(33, 5) ("Incomplete request, waiting for end of request line\n");
    return NULL;
    }
    if (r == -1) {
        return parseHttpRequestAbort(conn, "error:invalid-request");
    }
    /* Request line is valid here .. */
    *http_ver = HttpVersion(hp->v_maj, hp->v_min);

    /* This call scans the entire request, not just the headers */
    if (hp->v_maj > 0) {
        if ((req_sz = headersEnd(hp->buf, hp->bufsiz)) == 0) {
            debug(33, 5) ("Incomplete request, waiting for end of headers\n");
            return NULL;
        }
    } else {
        debug(33, 3) ("parseHttpRequest: Missing HTTP identifier\n");
        req_sz = HttpParserReqSz(hp);
    }

    /* We know the whole request is in hp->buf now */

    assert(req_sz <= (size_t) hp->bufsiz);
    /* Will the following be true with HTTP/0.9 requests? probably not .. */
    /* So the rest of the code will need to deal with '0'-byte headers (ie, none, so don't try parsing em) */
    assert(req_sz > 0);
    hp->hdr_end = req_sz - 1;
    hp->hdr_start = hp->req_end + 1;

    /* Enforce max_request_size */
    if (req_sz >= Config.maxRequestHeaderSize) {
        debug(33, 5) ("parseHttpRequest: Too large request\n");
        return parseHttpRequestAbort(conn, "error:request-too-large");
    }

    /* Set method_p */
    *method_p = HttpRequestMethod(&hp->buf[hp->m_start], &hp->buf[hp->m_end]);
    if (*method_p == METHOD_NONE) {
    /* XXX need a way to say "this many character length string" */
        debug(33, 1) ("clientParseRequestMethod: Unsupported method in request '%s'\n", hp->buf);
    /* XXX where's the method set for this error? */
        return parseHttpRequestAbort(conn, "error:unsupported-request-method");
    }

    /*
     * Process headers after request line
     * TODO: Use httpRequestParse here.
     */
    /* XXX this code should be modified to take a const char * later! */
    req_hdr = (char *) hp->buf + hp->req_end + 1;
    debug(33, 3) ("parseHttpRequest: req_hdr = {%s}\n", req_hdr);
    end = (char *) hp->buf + hp->hdr_end;
    debug(33, 3) ("parseHttpRequest: end = {%s}\n", end);

    if (strstr(req_hdr, "\r\r\n")) {
        debug(33, 1) ("WARNING: suspicious HTTP request contains double CR\n");
        return parseHttpRequestAbort(conn, "error:double-CR");
    }

    debug(33, 3) ("parseHttpRequest: prefix_sz = %d, req_line_sz = %d\n",
                  (int) HttpParserRequestLen(hp), HttpParserReqSz(hp));

    /* Ok, all headers are received */
    http = new ClientHttpRequest(conn);

    http->req_sz = HttpParserRequestLen(hp);
    result = ClientSocketContextNew(http);
    tempBuffer.data = result->reqbuf;
    tempBuffer.length = HTTP_REQBUF_SZ;

    ClientStreamData newServer = new clientReplyContext(http);
    ClientStreamData newClient = result;
    clientStreamInit(&http->client_stream, clientGetMoreData, clientReplyDetach,
                     clientReplyStatus, newServer, clientSocketRecipient,
                     clientSocketDetach, newClient, tempBuffer);

    debug(33, 5) ("parseHttpRequest: Request Header is\n%s\n",
                  (hp->buf) + hp->hdr_start);

    url = (char *)(hp->buf + hp->u_start);
    url_size = hp->u_end - hp->u_start + 1;

#if THIS_VIOLATES_HTTP_SPECS_ON_URL_TRANSFORMATION
/*url is not \0 terminated. this code if realy needed must rewritten*/
    if ((t = strchr(url, '#'))) /* remove HTML anchors */
        *t = '\0';

#endif

    /* Rewrite the URL in transparent or accelerator mode */
    if (conn->transparent()) {
        prepareTransparentURL(conn, http, url, url_size, req_hdr);
    } else if (conn->port->accel) {
         prepareAcceleratedURL(conn, http, url, url_size, req_hdr);
    } else if (internalCheckn(url, url_size)) {
        /* prepend our name & port */
        char *s = (char *)xmalloc(url_size + 16);
        memcpy(s, url, url_size);
        s[url_size] = '\0';
        http->uri = xstrdup(internalLocalUri(NULL, s));
        http->flags.accel = 1;
        xfree(s);
    }

    if (!http->uri) {
        /* No special rewrites have been applied above, use the
         * requested url. may be rewritten later, so make extra room */
        int u_sz = url_size + Config.appendDomainLen + 5;
        http->uri = (char *)xcalloc(u_sz, 1);
        memcpy(http->uri, url, url_size);
        http->uri[url_size]='\0';
    }

    setLogUri(http, http->uri);
    debug(33, 5) ("parseHttpRequest: Complete request received\n");
    result->flags.parsed_ok = 1;
    return result;
}
Received on Wed Feb 28 2007 - 11:47:17 MST

This archive was generated by hypermail pre-2.1.9 : Thu Mar 01 2007 - 12:00:02 MST