On Thursday, August 19, 2004 4:41 PM Neil Schemenauer wrote:
>
> On Wed, Aug 11, 2004 at 09:15:31PM -0400, Graham Fawcett wrote:
> >         # Propagate HTTP headers - copied from twisted_http.py
> >
> >         for title, header in msg.items():
> >             envname = title.replace('-', '_').upper()
> >             if title not in ('content-type', 'content-length'):
> >                 envname = "HTTP_" + envname
> >             environ[envname] = header
>
> [...]
>
> > In re-reading this code, I'm not sure why the exception is made for
> > HTTP_CONTENT_TYPE and HTTP_CONTENT_LENGTH. Not a big deal, but why run
> > the comparison for every header entry, just to get rid of them? Seems
> > wasteful; and I'd recommend we shorten this to
> >
> >         for title, header in msg.items():
> >             envname = title.replace('-', '_').upper()
> >             envname = "HTTP_" + envname
> >             environ[envname] = header
> >
> > in both medusa_http and twisted_http.
>
> Has anyone tried this?  I'm wondering if this is a safe change to
> make in the Quixote 1.x series.
>
>   Neil


No!

Um... This went in one ear and out the other when Graham first posted this,
but I don't think this is a good idea.

[Disclaimer, I haven't tried the patch.  I've only read the code]

The original code didn't "get rid of" the content-type and content-length
headers, it just skipped the mangling step of prepending the "HTTP_" to
them, inserting them as "CONTENT_TYPE" and "CONTENT_LENGTH" (instead of
"HTTP_CONTENT_TYPE" and "HTTP_CONTENT_LENGTH" which is what the patch will
cause)...  They aren't supposed to have the HTTP_ prefix.  They're 'real'
CGI environment variables.  The HTTP_ prefix is something that's done to all
the 'extra' headers that come across that the CGI spec doesn't specifically
address.


If we're looking to improve efficiency, something like this *might* be an
improvement:

         for title, header in msg.items():
             envname = title.replace('-', '_').upper()
             environ[envname] = header

         if envname.has_key('HTTP_CONTENT_TYPE'):
             environ['CONTENT_TYPE'] = environ['HTTP_CONTENT_TYPE']
             del environ['HTTP_CONTENT_TYPE']

         if envname.has_key('HTTP_CONTENT_LENGTH'):
             environ['CONTENT_LENGTH'] = environ['HTTP_CONTENT_LENGTH']
             del environ['HTTP_CONTENT_LENGTH']


.... but it sure is uglier.

I'd vote to just leave as it was, for my 2 cents,

Jason