httpd: memory hole in function addEnv() and more

[Vladimir N. Oleynik]

Dirk,

> Let's have a look into addEnv() from httpd.c
> 
> static void addEnv(const char *name_before_underline,
>             const char *name_after_underline, const char *value)
> {
>   char *s = NULL;
>   const char *underline;
> 
>   if (!value)
>     value = "";
>   underline = *name_after_underline ? "_" : "";
>   asprintf(&s, "%s%s%s=%s", name_before_underline, underline,
>                     name_after_underline, value);
>   if(s) {
>     putenv(s);
>   }
> }
> 
> ************
> 
> 1.) memory hole
> 
> Them memory alloced by asprintf() is never free'd.
> Solution: insert free(s) behind putenv(s).

No.
man putenv()
see libc incompatibility memory allocated sections.

> 2.) interface of addEnv()
> 
> addEnv() takes 2 parameters for bulding the variable name.
> I have analysed all callings and there is no need (including addEnvPort())
> to have 2 of them. And this makes the function and all callings smaller
> again:
> 
> static void addEnv(const char *name, const char *value)
> {
>   char *s = NULL;
>   asprintf(&s, "%s=%s", name, value ? value : "" );
>   if(s) {
>     putenv(s);
>     free(s);
>   }
> }
> 
> If we use this we need also some modification of addEnvPort()
> 
> static void addEnvPort(const char *name)
> {
>       char buf[16];
> 
>       sprintf(buf, "%u", config->port);
>       addEnv(name, buf);
> }
> 
> and the calls must change from
>       addEnvPort("REMOTE");
> to
>       addEnvPort("REMOTE_PORT");

Is not small. I test it.
"REMOTE_ADDR" + "REMOTE_PORT" have 24 bytes (with \0)
"REMOTE" "ADDR" "PORT" have 14 bytes, and addEnvPort() add
environ with and without "_".


> ************
> 
> 3.) usage of addEnv()
> In some source positions I found callings like
>       addEnv("SERVER",         "PROTOCOL", "HTTP/1.0");
> with 3 string constants.
> I think that we can save memory and cpu time if we change such lines into
>     putenv("SERVER_PROTOCOL=HTTP/1.0");
> 
> ************

See previous point.

> 4.)
> In my opinion it does not make sense:
>       addEnv("PATH",           "",         getenv("PATH"));

??


--w
vodz