You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Brian McCallister <br...@skife.org> on 2009/01/23 01:15:11 UTC
Re: patch for handling headers_in and headers_out as tables in
mod_lua
re-re-nudge :-)
On Wed, Jan 14, 2009 at 1:03 PM, Brian McCallister <br...@skife.org> wrote:
> re-nudge :-)
>
> On Tue, Jan 13, 2009 at 12:43 AM, Bertrand Mansion <bm...@mamasam.net> wrote:
>> +1
>>
>> Le 13 janv. 09 à 05:28, "Brian McCallister" <br...@skife.org> a écrit :
>>
>>> Nudge -- if there are no objections to the patch, would someone be
>>> kind enough to apply it?
>>>
>>> -Brian
>>>
>>> On Sun, Jan 11, 2009 at 8:20 PM, Brian McCallister <br...@skife.org>
>>> wrote:
>>>>
>>>> The attached patch changes headers_in and headers_out handling in
>>>> mod_lua as boxed userdata rather than functions.
>>>>
>>>> Basically we go from:
>>>>
>>>> -- OLD
>>>> function handle(r)
>>>> local host = r.headers_in("host")
>>>> r:puts(host)
>>>> end
>>>>
>>>> to
>>>>
>>>> -- NEW
>>>> function handle(r)
>>>> local host = r.headers_in['host']
>>>> r:puts(host)
>>>>
>>>> -- and can now modify them
>>>> r.headers_in['X-XX-Fake'] = 'rabbits!'
>>>> r.headers_out['wombat'] = 'lua now!'
>>>> end
>>>>
>>>> In order to treat tables idiommatically, it also corrects the
>>>> apl_push_apr_table(..) function to behave as a regular lua style push
>>>> function instead of a set function.
>>>>
>>>> -Brian
>>>>
>>
>
Re: patch for handling headers_in and headers_out as tables in
mod_lua
Posted by Bertrand Mansion <bm...@mamasam.net>.
On Sun, Mar 1, 2009 at 2:37 AM, Brian McCallister <br...@skife.org> wrote:
> On Fri, Feb 27, 2009 at 2:55 AM, Bertrand Mansion <bm...@mamasam.net> wrote:
>
>> And I find it weird that, now, to get 'document root', you have to
>> call r:document_root() instead of r.document_root as it was before.
>> That's because the dispatch is set like this:
>> apr_hash_set(dispatch, "document_root", APR_HASH_KEY_STRING,
>> makefun(&req_document_root, APL_REQ_FUNTYPE_LUACFUN, p));
>>
>> It could be just:
>> apr_hash_set(dispatch, "document_root", APR_HASH_KEY_STRING,
>> makefun(&req_content_encoding_field, APL_REQ_FUNTYPE_STRING,
>> p));
>
> Agreed, this is much nicer syntax. Applied this change as well. Thanks!
Hi Brian, the change does not compile on my box, I suggest this instead:
static char *req_document_root(request_rec *r)
{
const char *docroot = ap_document_root(r);
return apr_pstrdup(r->pool, docroot ? docroot : "");
}
Thanks,
--
Bertrand Mansion
Mamasam
Re: patch for handling headers_in and headers_out as tables in mod_lua
Posted by Brian McCallister <br...@skife.org>.
--
Please forgive typos, sent from phone.
On Mar 2, 2009, at 5:34 AM, Brian Akins <br...@akins.org> wrote:
> On 2/28/09 8:37 PM, "Brian McCallister" <br...@skife.org> wrote:
>
>>> It could be just:
>>> apr_hash_set(dispatch, "document_root", APR_HASH_KEY_STRING,
>>> makefun(&req_content_encoding_field,
>>> APL_REQ_FUNTYPE_STRING,
>>> p));
>>
>
>
> Also, couldn't we build the dispatch has once, and only once, and
> then just
> associated it with each apache2.request? This seems more efficient
> than
> building this array every time. It would also be nice to use a
> dispatch
> hash for "setters" as well.
Agreed. I'm thinking we can store the dispatch tables on the server
conf, that way we can allow them to be modified.
> --Brian
>
>
Re: patch for handling headers_in and headers_out as tables in
mod_lua
Posted by Brian Akins <br...@akins.org>.
On 2/28/09 8:37 PM, "Brian McCallister" <br...@skife.org> wrote:
>> It could be just:
>> apr_hash_set(dispatch, "document_root", APR_HASH_KEY_STRING,
>> makefun(&req_content_encoding_field, APL_REQ_FUNTYPE_STRING,
>> p));
>
Also, couldn't we build the dispatch has once, and only once, and then just
associated it with each apache2.request? This seems more efficient than
building this array every time. It would also be nice to use a dispatch
hash for "setters" as well.
--Brian
Re: patch for handling headers_in and headers_out as tables in
mod_lua
Posted by Brian McCallister <br...@skife.org>.
On Fri, Feb 27, 2009 at 2:55 AM, Bertrand Mansion <bm...@mamasam.net> wrote:
> By the way, for content-type, the dispatch function at the moment is:
> if (0 == apr_strnatcmp("content_type", key)) {
> const char *value = luaL_checkstring(L, 3);
> r->content_type = apr_pstrdup(r->pool, value);
> luaL_getmetatable(L, "Apache2.Request");
> lua_pushstring(L, value);
> lua_setfield(L, -2, "content_type");
> lua_pop(L, 1);
> return 0;
> }
> Shouldn't it use ap_set_content_type() instead ?
It should, in fact, thanks. Just checked in the change.
> And I find it weird that, now, to get 'document root', you have to
> call r:document_root() instead of r.document_root as it was before.
> That's because the dispatch is set like this:
> apr_hash_set(dispatch, "document_root", APR_HASH_KEY_STRING,
> makefun(&req_document_root, APL_REQ_FUNTYPE_LUACFUN, p));
>
> It could be just:
> apr_hash_set(dispatch, "document_root", APR_HASH_KEY_STRING,
> makefun(&req_content_encoding_field, APL_REQ_FUNTYPE_STRING,
> p));
Agreed, this is much nicer syntax. Applied this change as well. Thanks!
>
> --
> Bertrand Mansion
> Mamasam
>
Re: patch for handling headers_in and headers_out as tables in
mod_lua
Posted by Bertrand Mansion <bm...@mamasam.net>.
On Thu, Feb 5, 2009 at 11:14 PM, Brian Akins <br...@akins.org> wrote:
> On 2/5/09 1:51 PM, "Brian McCallister" <br...@skife.org> wrote:
>
>> Yep, Paul changed the internal impl to be less gross, but in doing so
>> changed the API, i changed the impl to be not gross and restored old
>> API.
>
> Okay I see it now.
>
> I may take a crack at a little performance tuning. Setting up the dispatch
> table every single time doesn't seem necessary. We should be able to just
> do that once at httpd start time.
>
>
> So, if I'm reading that correctly, this should work now as well??
>
> r.content_type
>
> And we would just hack up req_newindex to be able to do
> r.content_type = "application/bakins"
>
> I like the dispatch idea. I'll think about the same for newindex as well...
By the way, for content-type, the dispatch function at the moment is:
if (0 == apr_strnatcmp("content_type", key)) {
const char *value = luaL_checkstring(L, 3);
r->content_type = apr_pstrdup(r->pool, value);
luaL_getmetatable(L, "Apache2.Request");
lua_pushstring(L, value);
lua_setfield(L, -2, "content_type");
lua_pop(L, 1);
return 0;
}
Shouldn't it use ap_set_content_type() instead ?
And I find it weird that, now, to get 'document root', you have to
call r:document_root() instead of r.document_root as it was before.
That's because the dispatch is set like this:
apr_hash_set(dispatch, "document_root", APR_HASH_KEY_STRING,
makefun(&req_document_root, APL_REQ_FUNTYPE_LUACFUN, p));
It could be just:
apr_hash_set(dispatch, "document_root", APR_HASH_KEY_STRING,
makefun(&req_content_encoding_field, APL_REQ_FUNTYPE_STRING,
p));
--
Bertrand Mansion
Mamasam
Re: patch for handling headers_in and headers_out as tables in
mod_lua
Posted by Brian Akins <br...@akins.org>.
On 2/5/09 1:51 PM, "Brian McCallister" <br...@skife.org> wrote:
> Yep, Paul changed the internal impl to be less gross, but in doing so
> changed the API, i changed the impl to be not gross and restored old
> API.
Okay I see it now.
I may take a crack at a little performance tuning. Setting up the dispatch
table every single time doesn't seem necessary. We should be able to just
do that once at httpd start time.
So, if I'm reading that correctly, this should work now as well??
r.content_type
And we would just hack up req_newindex to be able to do
r.content_type = "application/bakins"
I like the dispatch idea. I'll think about the same for newindex as well...
Re: patch for handling headers_in and headers_out as tables in
mod_lua
Posted by Brian McCallister <br...@skife.org>.
On Thu, Feb 5, 2009 at 10:44 AM, Brian Akins <br...@akins.org> wrote:
> Is this in trunk? I don't see it, but I've been known to overlook stuff.
It is in trunk (httpd trunk)
>
> On 1/26/09 1:45 PM, "Brian McCallister" <br...@skife.org> wrote:
>
>> For anyone following, it has been applied :-)
>>
>
>>>>>>>
>>>>>>> -- NEW
>>>>>>> function handle(r)
>>>>>>> local host = r.headers_in['host']
>>>>>>> r:puts(host)
>>>>>>>
>>>>>>> -- and can now modify them
>>>>>>> r.headers_in['X-XX-Fake'] = 'rabbits!'
>>>>>>> r.headers_out['wombat'] = 'lua now!'
>>>>>>> end
>
> Wasn't this the way it used to be?
Yep, Paul changed the internal impl to be less gross, but in doing so
changed the API, i changed the impl to be not gross and restored old
API.
>
> --bakins
>
>
>
Re: patch for handling headers_in and headers_out as tables in
mod_lua
Posted by Brian Akins <br...@akins.org>.
Is this in trunk? I don't see it, but I've been known to overlook stuff.
On 1/26/09 1:45 PM, "Brian McCallister" <br...@skife.org> wrote:
> For anyone following, it has been applied :-)
>
>>>>>>
>>>>>> -- NEW
>>>>>> function handle(r)
>>>>>> local host = r.headers_in['host']
>>>>>> r:puts(host)
>>>>>>
>>>>>> -- and can now modify them
>>>>>> r.headers_in['X-XX-Fake'] = 'rabbits!'
>>>>>> r.headers_out['wombat'] = 'lua now!'
>>>>>> end
Wasn't this the way it used to be?
--bakins
Re: patch for handling headers_in and headers_out as tables in
mod_lua
Posted by Brian McCallister <br...@skife.org>.
For anyone following, it has been applied :-)
On Thu, Jan 22, 2009 at 4:15 PM, Brian McCallister <br...@skife.org> wrote:
> re-re-nudge :-)
>
> On Wed, Jan 14, 2009 at 1:03 PM, Brian McCallister <br...@skife.org> wrote:
>> re-nudge :-)
>>
>> On Tue, Jan 13, 2009 at 12:43 AM, Bertrand Mansion <bm...@mamasam.net> wrote:
>>> +1
>>>
>>> Le 13 janv. 09 à 05:28, "Brian McCallister" <br...@skife.org> a écrit :
>>>
>>>> Nudge -- if there are no objections to the patch, would someone be
>>>> kind enough to apply it?
>>>>
>>>> -Brian
>>>>
>>>> On Sun, Jan 11, 2009 at 8:20 PM, Brian McCallister <br...@skife.org>
>>>> wrote:
>>>>>
>>>>> The attached patch changes headers_in and headers_out handling in
>>>>> mod_lua as boxed userdata rather than functions.
>>>>>
>>>>> Basically we go from:
>>>>>
>>>>> -- OLD
>>>>> function handle(r)
>>>>> local host = r.headers_in("host")
>>>>> r:puts(host)
>>>>> end
>>>>>
>>>>> to
>>>>>
>>>>> -- NEW
>>>>> function handle(r)
>>>>> local host = r.headers_in['host']
>>>>> r:puts(host)
>>>>>
>>>>> -- and can now modify them
>>>>> r.headers_in['X-XX-Fake'] = 'rabbits!'
>>>>> r.headers_out['wombat'] = 'lua now!'
>>>>> end
>>>>>
>>>>> In order to treat tables idiommatically, it also corrects the
>>>>> apl_push_apr_table(..) function to behave as a regular lua style push
>>>>> function instead of a set function.
>>>>>
>>>>> -Brian
>>>>>
>>>
>>
>