[BUG] A string format defect in Nginx 0.8.48

[Maxim Dounin]

Hello!

On Tue, Aug 10, 2010 at 12:07:30PM +0800, Gary Hu wrote:

> I found a defect in function "ngx_vslprintf”which doesn't support right
> alignment format of a string, such as "%40s", as purported in the comments
> right above the function "ngx_sprintf" in file "src/core/ngx_string.c".

It is not clear why you think it should be supported and no 
support is a bug.  Both comments and code agree that it's not 
supported.

Additionally, it's not clear why do you think it should be 
supported for %s, but not for %*s, %V and %v.

And some patch comments just for completeness:

> The following is my suggested fix in function "ngx_vslprintf ":
> 
> 
> 
>             ......
> 
>            case 's':
> 
>               p = va_arg(args, u_char *);
> 
> 
> 
>                 //
> 
>                 // Added  by Gary Hu
> 
>                 //

You may want to use diff -u in the future for submitting patches.  

>                 size_t sublen;
> 
>                 size_t padlen;

Please follow nginx style when submitting patches.  In particular, 
please avoid declaring variables in function body.  And note that 
nobody stops you from reusing len for padlen, and slen for sublen.  
So basically declared variables aren't needed.

> 
>                 sublen = ngx_strlen(p);

This will likely cause SIGSEGV/SIGBUS for %*s as p isn't null 
terminated in this case. 

> 
> 
> 
>                 /* Add padding whitespaces to the output buffer to achieve
> right alignment*/
> 
>                 if (width > 0 && width > sublen)

Check "width > 0" is useless as both width and sublen are 
unsigned.

>                 {
> 
>                     padlen = ngx_min(((size_t) (last - buf)), width - sublen
> );
> 
>                     ngx_memset(buf, ' ', padlen);
> 
>                     buf += padlen;
> 
>                 }
> 
>                 //
> 
>                 // End added by Gary Hu
> 
>                 //

Maxim Dounin