[Mod_gzip] mod_gzip and strange vary headers

[mod_gzip@lists.over.net]

Hi Michael,

I have located the problem of sending incomplete Vary-headers:

the problem is not the function mod_gzip_generate_vary_header but the from
there calles function mod_gzip_strncpy.
this function gets as parameters: the dest string, the source string, the
length of the to be copied string.

mod_gzip_strncpy:
if((s1 != 0) && (s2 != 0)) {
!     while(*s2 != 0 && *s1 != 0 && len <= l) {
        *s1++ = *s2++;
        len++;
      }
*s1 = 0;

As you can see, in the while-loop always "*s1 "=0" is checked, but this
leads to the problem, that only  a certain count of bytes can be copied to s1. if
for example s1 points to "User-Agent" and this function is called to copy
"Accept-Language", then only 10 letters from "Accept-Language" are copied to s1
which leads to the value "Accept-Lan" that I have seen in my tests.
I guess, that *s1 != 0 is tested in order to garantuee that nothing can be
written beyond the length of s1 (I mean the string to which s1 points to).
This is ouf course ok, but leads in this case to wrong headers for Vary.

A possible solution:
I added a parameter max_l to mod_gzip_strncyp which is the max length of the
string s1 is pointing to ( MOD_GZIP_IMAP_MAXNAMELEN). Then I substituted the
check  *s1 != 0  with l < max_l

Now the correct headers are used for the Vary: header and I also tested
squid with this feature successfully.

I don't know if this is the best and correct solution, but in my opinion it
should be fixed fast, since without a fix caching of compressed content is
hardly possible (because of wrong vary information).

I append the patch and would like to know if this one  is ok.

regards,
alp


For the above fix, the patch would be:

*** mod_gzip-1.3.26.1a/mod_gzip.c       2002-10-01 09:29:49.000000000 +0200
--- mod_gzip-1.3.26.1a/mod_gzip_vary_patched.c  2004-03-23
13:10:28.000000000 +0100
***************
*** 1225,1235 ****
   return len;
  }

! int mod_gzip_strncpy(char *s1,char *s2,int l) {
    int len = 0;
-
    if((s1 != 0) && (s2 != 0)) {
!     while(*s2 != 0 && *s1 != 0 && len <= l) {
        *s1++ = *s2++;
        len++;
      }
--- 1225,1234 ----
   return len;
  }

! int mod_gzip_strncpy(char *s1,char *s2,int l,int max_l) {
    int len = 0;
    if((s1 != 0) && (s2 != 0)) {
!     while(*s2 != 0 && len <= l && len < max_l) {
        *s1++ = *s2++;
        len++;
      }
***************
*** 8072,8078 ****

    for(i=0;i<cfg->imap_total_entries;i++) {
      if(cfg->imap[i].type == MOD_GZIP_IMAP_ISREQHEADER) {
!      
mod_gzip_strncpy(name,cfg->imap[i].name,strstr(cfg->imap[i].name,":") - cfg->imap[i].name - 1);
        *((const char **)ap_push_array(ary)) = ap_pstrdup(p,name);
      }
    }
--- 8071,8077 ----

    for(i=0;i<cfg->imap_total_entries;i++) {
      if(cfg->imap[i].type == MOD_GZIP_IMAP_ISREQHEADER) {
!      
mod_gzip_strncpy(name,cfg->imap[i].name,strstr(cfg->imap[i].name,":") - cfg->imap[i].name - 1,MOD_GZIP_IMAP_MAXNAMELEN);
        *((const char **)ap_push_array(ary)) = ap_pstrdup(p,name);
      }
    }




-- 
+++ NEU bei GMX und erstmalig in Deutschland: TÜV-geprüfter Virenschutz +++
100% Virenerkennung nach Wildlist. Infos: http://www.gmx.net/virenschutz