[PATCH] Fix a memory invalid read issue in ngx_http_gzip_ok

[Maxim Dounin]

Hello!

On Thu, Dec 22, 2011 at 09:37:46PM +0800, agentzh wrote:

> On Thu, Dec 22, 2011 at 9:21 PM, Maxim Dounin <mdounin at mdounin.ru> wrote:
> >
> > As I already said more than once, I don't think we should.  If
> > it's needed - it means something else needs fixing.  We do support
> > arbitrary modification of headers passed to upstream servers, and
> > this should be enough.
> >
> > But, actually, nobody stops you from adding headers correctly,
> > i.e. null-terminated.
> >
> 
> Creating null-terminated buffers ourselves often means extra memory
> allocations and data copying because ngx_str_t does not require a C
> string at all ;)

Yes, this is exactly the reason why headers in r->headers_in are 
null-terminated (as well as e.g. arguments in config parsing 
functions).  This saves memory allocations and data copying 
if/when these strings will be used when null-terminated strings 
are required for external interfaces (and simplifies some internal 
code as well).

In typical (and the only one as of vanilla nginx) case of headers 
being read from client it costs nothing.  In other cases it 
usually means reserving another byte at configuration stage, which 
isn't something overwhelming either.

> Is it really so hard to just add a dead simple check there? ;) It does
> not break the current behavior of the Nginx core at all. If there's
> any other issues as you've mentioned, I'm very willing to help
> preparing patches :)

Non-null-terminated headers in r->headers_in *will* break nginx, 
ngx_http_gzip_ok() isn't the only place which relies on 
r->headers_in headers being null-terminated.  Take a look e.g. at 
ngx_http_dav_module.c.

Maxim Dounin