librelist archives

« back to archive

patch

patch

From:
Josh Forman
Date:
2011-05-18 @ 23:50
I was trying out wsgid with a Pylons application I wrote a while ago.  I 
was having session problems (I use Beaker's session middleware) and 
realized that it was not seeing the HTTP headers in the format it was 
looking for.

You're code to copy headers was generating things like HTTP_user-agent 
which is very unexpected.  This patch ought to take care of any headers 
that would take the form HTTP_WHATEVER, though it probably needs some more
work.  I'm sure I can toss that together pretty quick though.

diff --git a/wsgid/core/wsgid.py b/wsgid/core/wsgid.py
index 023639e..89e2f2d 100644
--- a/wsgid/core/wsgid.py
+++ b/wsgid/core/wsgid.py
@@ -146,6 +146,7 @@ class Wsgid(object):
       if header[0] in ('X', 'x'):
         environ[header] = str(value)
       else:
+        header = header.upper().replace('-','_')
         environ['HTTP_%s' % header] = str(value)
 
     return environ

Re: [wsgid] patch

From:
Dalton Barreto
Date:
2011-05-19 @ 01:23
2011/5/18 Josh Forman <josh@yoshrote.com>:
> I was trying out wsgid with a Pylons application I wrote a while ago.  I
was having session problems (I use Beaker's session middleware) and 
realized that it was not seeing the HTTP headers in the format it was 
looking for.
>
> You're code to copy headers was generating things like HTTP_user-agent 
which is very unexpected.  This patch ought to take care of any headers 
that would take the form HTTP_WHATEVER, though it probably needs some more
work.  I'm sure I can toss that together pretty quick though.
>

Thank you very much for your time trying wsgid. Tell me more about how
is the initialization of a Pylons app, so I can write a wsgid
AppLoader [1] and integrate it into mainline code.

> diff --git a/wsgid/core/wsgid.py b/wsgid/core/wsgid.py
> index 023639e..89e2f2d 100644
> --- a/wsgid/core/wsgid.py
> +++ b/wsgid/core/wsgid.py
> @@ -146,6 +146,7 @@ class Wsgid(object):
>       if header[0] in ('X', 'x'):
>         environ[header] = str(value)
>       else:
> +        header = header.upper().replace('-','_')
>         environ['HTTP_%s' % header] = str(value)
>
>     return environ
>
>


Great! Can you point the part of the Beaker's code that is getting the
wrong HTTP Header? Just to take a look.

I think that we don't need the "upper()" part, according to the
RFC[2], HTTP header names should be case insensitive, so it's up to
the applications to get the name right.

Actually I didn't converted "-" to "_" because I thought that
preserving the "original" name would be the best, but sincerely I
don't know if it's the best to do. =) I will take a closer look at
this.

Thank you very much and don't hesitate do fork the wsgid repo on
github [3] if you have more contributions to make.

[1] http://wsgid.com/static/docs/current/html/internals.html#app-loaders
[2] http://www.w3.org/Protocols/rfc2616/rfc2616-sec4.html#sec4.2
[3] https://github.com/daltonmatos/wsgid


-- 
Dalton Barreto
http://daltonmatos.wordpress.com
http://wsgid.com

Re: [wsgid] patch

From:
Josh Forman
Date:
2011-05-19 @ 02:00
On May 18, 2011, at 9:23 PM, Dalton Barreto wrote:

> 2011/5/18 Josh Forman <josh@yoshrote.com>:
>> I was trying out wsgid with a Pylons application I wrote a while ago.  
I was having session problems (I use Beaker's session middleware) and 
realized that it was not seeing the HTTP headers in the format it was 
looking for.
>> 
>> You're code to copy headers was generating things like HTTP_user-agent 
which is very unexpected.  This patch ought to take care of any headers 
that would take the form HTTP_WHATEVER, though it probably needs some more
work.  I'm sure I can toss that together pretty quick though.
>> 
> 
> Thank you very much for your time trying wsgid. Tell me more about how
> is the initialization of a Pylons app, so I can write a wsgid
> AppLoader [1] and integrate it into mainline code.
> 
I have a simple bootstrap script that i stored seperate from the 
application's package, but I moved it inside to make it easier to address 
using the --wsgi-app parameter on wsgid.  the script looks like this:

from paste.script.util.logging_config import fileConfig
fileConfig("/etc/pylonsapp.ini")
from paste.deploy import loadapp
application = loadapp("config:/etc/pylonsapp.ini")

I'm not sure if i even need the first two lines anymore because that was 
needed to get logging working correctly with apache's mod_wsgi.  All an 
AppLoader for pylons needs should be the call to paste.deploy.loadapp 
which should also work to load any other paste based framework such as 
turbogears.

>> diff --git a/wsgid/core/wsgid.py b/wsgid/core/wsgid.py
>> index 023639e..89e2f2d 100644
>> --- a/wsgid/core/wsgid.py
>> +++ b/wsgid/core/wsgid.py
>> @@ -146,6 +146,7 @@ class Wsgid(object):
>>       if header[0] in ('X', 'x'):
>>         environ[header] = str(value)
>>       else:
>> +        header = header.upper().replace('-','_')
>>         environ['HTTP_%s' % header] = str(value)
>> 
>>     return environ
>> 
>> 
> 
> 
> Great! Can you point the part of the Beaker's code that is getting the
> wrong HTTP Header? Just to take a look.
beaker.session
in SessionObject._session
   539	    def _session(self):
   540	        """Lazy initial creation of session object"""
   541	        if self.__dict__['_sess'] is None:
   542	            params = self.__dict__['_params']
   543	            environ = self.__dict__['_environ']
   544	            self.__dict__['_headers'] = req = {'cookie_out':None}
   545	            req['cookie'] = environ.get('HTTP_COOKIE')
   546	            if params.get('type') == 'cookie':
   547	                self.__dict__['_sess'] = CookieSession(req, **params)
   548	            else:
   549	                self.__dict__['_sess'] = Session(req, use_cookies=True,
   550	                                                 **params)
   551	        return self.__dict__['_sess']

This was from beaker 1.5.4, which is their latest release on pypi

Every request was marked as being a new session since it was looking for 
HTTP_COOKIE exactly.

> 
> I think that we don't need the "upper()" part, according to the
> RFC[2], HTTP header names should be case insensitive, so it's up to
> the applications to get the name right.
> 
> Actually I didn't converted "-" to "_" because I thought that
> preserving the "original" name would be the best, but sincerely I
> don't know if it's the best to do. =) I will take a closer look at
> this.
Not sure if I went overkill on that myself, but it made the headers look 
more like I was getting from mod_wsgi, which is apparently how Beaker was 
designed too.  It seems like an assumption carried over from CGI. 
http://en.wikipedia.org/wiki/Common_Gateway_Interface
> 
> Thank you very much and don't hesitate do fork the wsgid repo on
> github [3] if you have more contributions to make.
> 
> [1] http://wsgid.com/static/docs/current/html/internals.html#app-loaders
> [2] http://www.w3.org/Protocols/rfc2616/rfc2616-sec4.html#sec4.2
> [3] https://github.com/daltonmatos/wsgid
> 
> 
> -- 
> Dalton Barreto
> http://daltonmatos.wordpress.com
> http://wsgid.com

Re: [wsgid] patch

From:
Dalton Barreto
Date:
2011-05-20 @ 13:46
2011/5/18 Josh Forman <josh@yoshrote.com>:

> I have a simple bootstrap script that i stored seperate from the 
application's package, but I moved it inside to make it easier to address 
using the --wsgi-app parameter on wsgid.  the script looks like this:
>
> from paste.script.util.logging_config import fileConfig
> fileConfig("/etc/pylonsapp.ini")
> from paste.deploy import loadapp
> application = loadapp("config:/etc/pylonsapp.ini")
>
> I'm not sure if i even need the first two lines anymore because that was
needed to get logging working correctly with apache's mod_wsgi.  All an 
AppLoader for pylons needs should be the call to paste.deploy.loadapp 
which should also work to load any other paste based framework such as 
turbogears.
>

This ti great! But one quick question: Do you always need an external
config file do load an pylons app? If it is true, there is no way
today to write an wsgid AppLoader, as we can't pass parameters to the
Loaders for now. I would have to think some more about this.


> beaker.session
> in SessionObject._session
>   539      def _session(self):
>   540          """Lazy initial creation of session object"""
>   541          if self.__dict__['_sess'] is None:
>   542              params = self.__dict__['_params']
>   543              environ = self.__dict__['_environ']
>   544              self.__dict__['_headers'] = req = {'cookie_out':None}
>   545              req['cookie'] = environ.get('HTTP_COOKIE')
>   546              if params.get('type') == 'cookie':
>   547                  self.__dict__['_sess'] = CookieSession(req, **params)
>   548              else:
>   549                  self.__dict__['_sess'] = Session(req, use_cookies=True,
>   550                                                   **params)
>   551          return self.__dict__['_sess']
>
> This was from beaker 1.5.4, which is their latest release on pypi
>
> Every request was marked as being a new session since it was looking for
HTTP_COOKIE exactly.
>

I see. Indeed it is looking for an all-upper-case header name. Look at
how trac [1] deals with "HTTP_" headers:

640     def _parse_headers(self):
641	        headers = [(name[5:].replace('_', '-').lower(), value)
642	                   for name, value in self.environ.items()
643	                   if name.startswith('HTTP_')]

That is, it lowers all headers that starts with "HTTP_". The
"replace('_', '-')" part I think was just a design decision, just to
be able to get all headers always using "-", and not a mix of "_" and
"-". This code is in trac.web.api.py [2]

Maybe we could contact the beaker's developer and suggest them to do a
lower() on the headers? This way they will be more RFC compliant, what
do you think? Can you talk do them?

>> I think that we don't need the "upper()" part, according to the
>> RFC[2], HTTP header names should be case insensitive, so it's up to
>> the applications to get the name right.
>>
>> Actually I didn't converted "-" to "_" because I thought that
>> preserving the "original" name would be the best, but sincerely I
>> don't know if it's the best to do. =) I will take a closer look at
>> this.
> Not sure if I went overkill on that myself, but it made the headers look
more like I was getting from mod_wsgi, which is apparently how Beaker was 
designed too.  It seems like an assumption carried over from CGI. 
http://en.wikipedia.org/wiki/Common_Gateway_Interface

Agreed. I sincerely don't know about the "_"/"-" part, but definitely
the applications should look for headers in a case-insensitive way.
That's how the RFC specifies HTTP Headers.

[1] http://trac.edgewall.org/
[2] http://trac.edgewall.org/browser//trunk/trac/web/api.py#L640

Thanks,

-- 
Dalton Barreto
http://daltonmatos.wordpress.com
http://wsgid.com

Re: [wsgid] patch

From:
Josh Forman
Date:
2011-05-20 @ 20:54
On May 20, 2011, at 9:46 AM, Dalton Barreto wrote:

> 2011/5/18 Josh Forman <josh@yoshrote.com>:
> 
>> I have a simple bootstrap script that i stored seperate from the 
application's package, but I moved it inside to make it easier to address 
using the --wsgi-app parameter on wsgid.  the script looks like this:
>> 
>> from paste.script.util.logging_config import fileConfig
>> fileConfig("/etc/pylonsapp.ini")
>> from paste.deploy import loadapp
>> application = loadapp("config:/etc/pylonsapp.ini")
>> 
>> I'm not sure if i even need the first two lines anymore because that 
was needed to get logging working correctly with apache's mod_wsgi.  All 
an AppLoader for pylons needs should be the call to paste.deploy.loadapp 
which should also work to load any other paste based framework such as 
turbogears.
>> 
> 
> This ti great! But one quick question: Do you always need an external
> config file do load an pylons app? If it is true, there is no way
> today to write an wsgid AppLoader, as we can't pass parameters to the
> Loaders for now. I would have to think some more about this.
> 
It does require a config file always.  The way paste works though is that 
you can configure what package to use as a server which is a way to change
between paste's builtin webserver, flup, CherryPy or Spawning (those are 
all the servers i've seen used with paste that way).  That is probably the
easiest way for the various frameworks that rely on paste, but I'm sure 
there are other ways that could be tried.  Every other WSGI server I've 
used requires a bootstrap file like the one above.
> 
>> beaker.session
>> in SessionObject._session
>>   539      def _session(self):
>>   540          """Lazy initial creation of session object"""
>>   541          if self.__dict__['_sess'] is None:
>>   542              params = self.__dict__['_params']
>>   543              environ = self.__dict__['_environ']
>>   544              self.__dict__['_headers'] = req = {'cookie_out':None}
>>   545              req['cookie'] = environ.get('HTTP_COOKIE')
>>   546              if params.get('type') == 'cookie':
>>   547                  self.__dict__['_sess'] = CookieSession(req, **params)
>>   548              else:
>>   549                  self.__dict__['_sess'] = Session(req, use_cookies=True,
>>   550                                                   **params)
>>   551          return self.__dict__['_sess']
>> 
>> This was from beaker 1.5.4, which is their latest release on pypi
>> 
>> Every request was marked as being a new session since it was looking 
for HTTP_COOKIE exactly.
>> 
> 
> I see. Indeed it is looking for an all-upper-case header name. Look at
> how trac [1] deals with "HTTP_" headers:
> 
> 640     def _parse_headers(self):
> 641	        headers = [(name[5:].replace('_', '-').lower(), value)
> 642	                   for name, value in self.environ.items()
> 643	                   if name.startswith('HTTP_')]
> 
> That is, it lowers all headers that starts with "HTTP_". The
> "replace('_', '-')" part I think was just a design decision, just to
> be able to get all headers always using "-", and not a mix of "_" and
> "-". This code is in trac.web.api.py [2]
> 
> Maybe we could contact the beaker's developer and suggest them to do a
> lower() on the headers? This way they will be more RFC compliant, what
> do you think? Can you talk do them?
> 
>>> I think that we don't need the "upper()" part, according to the
>>> RFC[2], HTTP header names should be case insensitive, so it's up to
>>> the applications to get the name right.
>>> 
>>> Actually I didn't converted "-" to "_" because I thought that
>>> preserving the "original" name would be the best, but sincerely I
>>> don't know if it's the best to do. =) I will take a closer look at
>>> this.
>> Not sure if I went overkill on that myself, but it made the headers 
look more like I was getting from mod_wsgi, which is apparently how Beaker
was designed too.  It seems like an assumption carried over from CGI. 
http://en.wikipedia.org/wiki/Common_Gateway_Interface
> 
> Agreed. I sincerely don't know about the "_"/"-" part, but definitely
> the applications should look for headers in a case-insensitive way.
> That's how the RFC specifies HTTP Headers.
> 
> [1] http://trac.edgewall.org/
> [2] http://trac.edgewall.org/browser//trunk/trac/web/api.py#L640
> 
> Thanks,
> 
> -- 
> Dalton Barreto
> http://daltonmatos.wordpress.com
> http://wsgid.com

Re: [wsgid] patch

From:
Dalton Barreto
Date:
2011-05-21 @ 17:03
2011/5/20 Josh Forman <josh@yoshrote.com>:

> It does require a config file always.  The way paste works though is 
that you can configure what package to use as a server which is a way to 
change between paste's builtin webserver, flup, CherryPy or Spawning 
(those are all the servers i've seen used with paste that way).  That is 
probably the easiest way for the various frameworks that rely on paste, 
but I'm sure there are other ways that could be tried.  Every other WSGI 
server I've used requires a bootstrap file like the one above.

Josh, I've created a ticket to implement Pylons support on wsgid [1].

Although beaker still need to get HTTP headers in a case-insensitive
way to work properly with wsgid.

[1] http://dev.wsgid.com/ticket/5

Thanks.
-- 
Dalton Barreto
http://daltonmatos.wordpress.com
http://wsgid.com