Re: Change RAD_REPLY item in rlm_perl, not add a new pair
On Wednesday 16 August 2006 18:09, Alex French wrote:
> Boina,
>
> That works fine for me (patching against a clean 1.1.2 tree) I've only
> tested == and := operators but they seem fine.
>
> Only one point to note; if you do not include an element in the hash with
> the same name as the attribute ( e.g. due to a typo or just a
> misconfiguration), the server hangs completely the first time something
> gets passed through the perl module and needs a kill -9 to stop it. I know
> you can't protect people against their own configuration errors, but
> perhaps it would be nicer to log an error (or just ignore the attribute).
Thanks for suggestion i will correct this matter.
>
> Anyway, thanks very much for the patch!
>
> Thanks,
>
> On 15/08/06, Boian Jordanov <[EMAIL PROTECTED]> wrote:
> > On Monday 14 August 2006 21:27, Alex French wrote:
> > > Boian,
> > >
> > > Thanks, if you have a patch that actually implements the hash for the
> > > operator etc, that would be great (in fact, why not just submit it as a
> > > feature). If it's just to change the operator hardcoded in rlm_perl.c,
> > > that's fine, I have that recompiled and installed at the moment,
> >
> > Yep, i have the patch that implements the operator with hash ref.
> > Test it and if you like it i will submit it in CVS HEAD.
> >
> > For example to change Operator for Framed-MTU
> >
> > $hash{'Framed-MTU'} = "100";
> > $hash{'Operator'} = "==";
> > $RAD_REPLY{'Framed-MTU'} = \%hash;
> >
> >
> > --
> > Best Regards,
> > Boian Jordanov
> > SNE
> > Orbitel - Next Generation Telecom
> > tel. +359 2 4004 723
> > tel. +359 2 4004 002
> >
> >
> > -
> > List info/subscribe/unsubscribe? See
> > http://www.freeradius.org/list/users.html
--
Best Regards,
Boian Jordanov
SNE
Orbitel - Next Generation Telecom
tel. +359 2 4004 723
tel. +359 2 4004 002
-
List info/subscribe/unsubscribe? See http://www.freeradius.org/list/users.html
Re: Change RAD_REPLY item in rlm_perl, not add a new pair
Boina,That works fine for me (patching against a clean 1.1.2 tree) I've only tested == and := operators but they seem fine.Only
one point to note; if you do not include an element in the hash with
the same name as the attribute (
e.g. due to a typo or just a misconfiguration), the server hangs
completely the first time something gets passed through the perl module
and needs a kill -9 to stop it. I know you can't protect people against
their own configuration errors, but perhaps it would be nicer to log an
error (or just ignore the attribute).
Anyway, thanks very much for the patch!Thanks,On 15/08/06, Boian Jordanov <
[EMAIL PROTECTED]> wrote:On Monday 14 August 2006 21:27, Alex French wrote:
> Boian,>> Thanks, if you have a patch that actually implements the hash for the> operator etc, that would be great (in fact, why not just submit it as a> feature). If it's just to change the operator hardcoded in rlm_perl.c,
> that's fine, I have that recompiled and installed at the moment,Yep, i have the patch that implements the operator with hash ref.Test it and if you like it i will submit it in CVS HEAD.For example to change Operator for Framed-MTU
$hash{'Framed-MTU'} = "100";$hash{'Operator'} = "==";$RAD_REPLY{'Framed-MTU'} = \%hash;--Best Regards,Boian JordanovSNEOrbitel - Next Generation Telecomtel. +359 2 4004 723
tel. +359 2 4004 002-List info/subscribe/unsubscribe? See http://www.freeradius.org/list/users.html
-
List info/subscribe/unsubscribe? See http://www.freeradius.org/list/users.html
Re: Change RAD_REPLY item in rlm_perl, not add a new pair
On Monday 14 August 2006 21:27, Alex French wrote:
> Boian,
>
> Thanks, if you have a patch that actually implements the hash for the
> operator etc, that would be great (in fact, why not just submit it as a
> feature). If it's just to change the operator hardcoded in rlm_perl.c,
> that's fine, I have that recompiled and installed at the moment,
Yep, i have the patch that implements the operator with hash ref.
Test it and if you like it i will submit it in CVS HEAD.
For example to change Operator for Framed-MTU
$hash{'Framed-MTU'} = "100";
$hash{'Operator'} = "==";
$RAD_REPLY{'Framed-MTU'} = \%hash;
--
Best Regards,
Boian Jordanov
SNE
Orbitel - Next Generation Telecom
tel. +359 2 4004 723
tel. +359 2 4004 002
Index: rlm_perl.c
===
RCS file: /source/radiusd/src/modules/rlm_perl/rlm_perl.c,v
retrieving revision 1.13.4.7
diff -u -r1.13.4.7 rlm_perl.c
--- rlm_perl.c 27 Apr 2006 17:35:44 - 1.13.4.7
+++ rlm_perl.c 15 Aug 2006 12:22:32 -
@@ -920,22 +920,49 @@
*/
static int get_hv_content(HV *my_hv, VALUE_PAIR **vp)
{
- SV *res_sv, **av_sv;
+ SV *res_sv, **av_sv, **operator_sv, **sv;
+ HV *hv;
AV *av;
- char *key;
+ char *key,*tmp,*ptr;
+ char buf[MAX_STRING_LEN];
I32 key_len, len, i, j;
int ret=0;
+ LRAD_TOKEN token, operator = T_EOL;
for (i = hv_iterinit(my_hv); i > 0; i--) {
- res_sv = hv_iternextsv(my_hv,&key,&key_len);
- if (SvROK(res_sv) && (SvTYPE(SvRV(res_sv)) == SVt_PVAV)) {
+
+ res_sv = hv_iternextsv(my_hv,&key,&key_len);
+
+ if (SvROK(res_sv) && (SvTYPE(SvRV(res_sv)) == SVt_PVHV)) {
+ hv = (HV*)SvRV(res_sv);
+ sv = hv_fetch(hv,key,key_len,FALSE);
+
+ operator_sv = hv_fetch(hv,"Operator", strlen("Operator"), FALSE);
+
+ if (SvOK(*operator_sv))
+ ptr = SvPV_nolen(*operator_sv);
+
+ operator = gettoken(&ptr, buf, sizeof(buf));
+ /*
+ * Check Operator
+ *
+ */
+ if (operator <= T_EOL ) {
+ radlog(L_ERR,"Invalid Operator for attribute %s", key);
+ operator = T_OP_EQ;
+ }
+ ret = pairadd_sv(vp,key, *sv, operator);
+ }
+ else if (SvROK(res_sv) && (SvTYPE(SvRV(res_sv)) == SVt_PVAV)) {
av = (AV*)SvRV(res_sv);
len = av_len(av);
for (j = 0; j <= len; j++) {
av_sv = av_fetch(av, j, 0);
ret = pairadd_sv(vp, key, *av_sv, T_OP_ADD) + ret;
}
- } else ret = pairadd_sv(vp, key, res_sv, T_OP_EQ) + ret;
+ } else {
+ ret = pairadd_sv(vp, key, res_sv, T_OP_EQ) + ret;
+ }
}
return ret;
@@ -1051,6 +1078,7 @@
if ((get_hv_content(rad_reply_hv, &vp)) > 0 ) {
+ //pairfree(&request->reply->vps);
pairmove(&request->reply->vps, &vp);
pairfree(&vp);
}
-
List info/subscribe/unsubscribe? See http://www.freeradius.org/list/users.html
Re: Change RAD_REPLY item in rlm_perl, not add a new pair
On Monday 14 August 2006 21:27, Alex French wrote: > Boian, > > Thanks, if you have a patch that actually implements the hash for the > operator etc, that would be great (in fact, why not just submit it as a > feature). If it's just to c- List info/subscribe/unsubscribe? See http://www.freeradius.org/list/users.html
Re: Change RAD_REPLY item in rlm_perl, not add a new pair
On 13/08/06, Boian Jordanov <[EMAIL PROTECTED]> wrote: On Friday 11 August 2006 20:18, Alex French wrote:> Hi,>> Does anyone know if anything was done on the issue below? I'm looking for> this functionality too, and I'd prefer not to have to recompile the module > if the feature is available in HEAD or similar (although I can't see> that...).No sorry,but i can give you a patch if you want off course.Boian,Thanks, if you have a patch that actually implements the hash for the operator etc, that would be great (in fact, why not just submit it as a feature). If it's just to change the operator hardcoded in rlm_perl.c, that's fine, I have that recompiled and installed at the moment, Alex - List info/subscribe/unsubscribe? See http://www.freeradius.org/list/users.html
Re: Change RAD_REPLY item in rlm_perl, not add a new pair
On Friday 11 August 2006 20:18, Alex French wrote:
> Hi,
>
> Does anyone know if anything was done on the issue below? I'm looking for
> this functionality too, and I'd prefer not to have to recompile the module
> if the feature is available in HEAD or similar (although I can't see
> that...).
No sorry,
but i can give you a patch if you want off course.
>
> Thanks,
>
> Alex
>
> On 22/06/06, Kenneth Marshall <[EMAIL PROTECTED]> wrote:
> > On Thu, Jun 22, 2006 at 09:58:54AM +0300, Boian Jordanov wrote:
> > > Maybe passing a HASH ref for hash which contains the Operator key and
> >
> > the vp
> >
> > > item too will be a good idea. For example
> > >
> > > $hash{'Tunnel-Id'} = "visitor";
> > > $hash{'Operator'} = ":=";
> > > $RAD_REPLY{'Tunnel-Id'} = \%hash;
> > >
> > > This way we will not change existing behavior.
> >
> > I like this. One key feature missing in rlm_perl was the ability
> > to substitute values in attribute pairs, not just add a new one.
> >
> > Ken
> > -
> > List info/subscribe/unsubscribe? See
> > http://www.freeradius.org/list/users.html
--
Best Regards,
Boian Jordanov
SNE
Orbitel - Next Generation Telecom
tel. +359 2 4004 723
tel. +359 2 4004 002
-
List info/subscribe/unsubscribe? See http://www.freeradius.org/list/users.html
Re: Change RAD_REPLY item in rlm_perl, not add a new pair
Hi,Does anyone know if anything was done on the issue below?
I'm looking for this functionality too, and I'd prefer not to have to
recompile the module if the feature is available in HEAD or similar
(although I can't see that...).
Thanks,AlexOn 22/06/06, Kenneth Marshall <[EMAIL PROTECTED]> wrote:
On Thu, Jun 22, 2006 at 09:58:54AM +0300, Boian Jordanov wrote:> Maybe passing a HASH ref for hash which contains the Operator key and the vp> item too will be a good idea. For example>> $hash{'Tunnel-Id'} = "visitor";
> $hash{'Operator'} = ":=";> $RAD_REPLY{'Tunnel-Id'} = \%hash;>> This way we will not change existing behavior.>I like this. One key feature missing in rlm_perl was the ability
to substitute values in attribute pairs, not just add a new one.Ken-List info/subscribe/unsubscribe? See http://www.freeradius.org/list/users.html
-
List info/subscribe/unsubscribe? See http://www.freeradius.org/list/users.html
Re: Change RAD_REPLY item in rlm_perl, not add a new pair
On Thu, Jun 22, 2006 at 09:58:54AM +0300, Boian Jordanov wrote:
> Maybe passing a HASH ref for hash which contains the Operator key and the vp
> item too will be a good idea. For example
>
> $hash{'Tunnel-Id'} = "visitor";
> $hash{'Operator'} = ":=";
> $RAD_REPLY{'Tunnel-Id'} = \%hash;
>
> This way we will not change existing behavior.
>
I like this. One key feature missing in rlm_perl was the ability
to substitute values in attribute pairs, not just add a new one.
Ken
-
List info/subscribe/unsubscribe? See http://www.freeradius.org/list/users.html
Re: Change RAD_REPLY item in rlm_perl, not add a new pair
On Tuesday 20 June 2006 19:05, Kenneth Marshall wrote:
> On Tue, Jun 20, 2006 at 11:05:04AM +0200, Bj?rn Mork wrote:
> > Kenneth Marshall <[EMAIL PROTECTED]> writes:
> > > I am trying to use rlm_perl to append a number to one
> > > member of the reply packet using rlm_perl and the %RAD_REPLY
> > > hash. I am running freeradius-1.1.1.
> >
> > I don't think you can do that with rlm_perl. The inability to specify
> > operator is limiting.
> >
> > rlm_perl will choose T_OP_ADD if the hash value is an array reference.
> > Otherwise it defaults to T_OP_EQ when creating the lists of vps out of
> > the %RAD_REPLY and %RAD_CHECK hashes. There is no way to specify
> > T_OP_SET, which is the functionality you need.
> >
> > I would really love to improve this, but I can't think of any nice way
> > to do it. If one were to add operators to the perl hashes, how could
> > that be done? Adding an additional set of hashes, mapping attribute
> > names to operators? Adding "magic" operator strings to either key or
> > value of the current hashes?
> >
> > I'm afraid that noen of these will be backwards compatible with
> > existing perl scripts using rlm_perl.
> >
> > Another possibility would be to let rlm_perl assume T_OP_SET for
> > modified attributes (unless they are array references). This makes
> > sense to me (and to you it seems, since that's what you expected :-).
> > But I dont't know of an effecient way to implement this. You'd
> > probably have to walk through the lists, comparing the values of all
> > attributes. Unless you tie the hashes to some class implementing the
> > necessarry logic in its STORE function? That might be a possibilty...
>
> I agree with you. It makes more sense to modify attributes that are
> changed instead of appending a new pair which will be discarded in most
> cases. Is there an easy way to tie a "modified" flag to each attribute
> that could be changed to indicate a modification. Or use something
> like a composite key "attribute+operator" with a missing operator
> defaulting to current behavior. This would allow people to use ":="
> when needed and old code should still work correctly.
Maybe passing a HASH ref for hash which contains the Operator key and the vp
item too will be a good idea. For example
$hash{'Tunnel-Id'} = "visitor";
$hash{'Operator'} = ":=";
$RAD_REPLY{'Tunnel-Id'} = \%hash;
This way we will not change existing behavior.
--
Best Regards,
Boian Jordanov
SNE
Orbitel - Next Generation Telecom
tel. +359 2 4004 723
tel. +359 2 4004 002
-
List info/subscribe/unsubscribe? See http://www.freeradius.org/list/users.html
Re: Change RAD_REPLY item in rlm_perl, not add a new pair
On Tue, Jun 20, 2006 at 11:05:04AM +0200, Bj?rn Mork wrote: > Kenneth Marshall <[EMAIL PROTECTED]> writes: > > > I am trying to use rlm_perl to append a number to one > > member of the reply packet using rlm_perl and the %RAD_REPLY > > hash. I am running freeradius-1.1.1. > > I don't think you can do that with rlm_perl. The inability to specify > operator is limiting. > > rlm_perl will choose T_OP_ADD if the hash value is an array reference. > Otherwise it defaults to T_OP_EQ when creating the lists of vps out of > the %RAD_REPLY and %RAD_CHECK hashes. There is no way to specify > T_OP_SET, which is the functionality you need. > > I would really love to improve this, but I can't think of any nice way > to do it. If one were to add operators to the perl hashes, how could > that be done? Adding an additional set of hashes, mapping attribute > names to operators? Adding "magic" operator strings to either key or > value of the current hashes? > > I'm afraid that noen of these will be backwards compatible with > existing perl scripts using rlm_perl. > > Another possibility would be to let rlm_perl assume T_OP_SET for > modified attributes (unless they are array references). This makes > sense to me (and to you it seems, since that's what you expected :-). > But I dont't know of an effecient way to implement this. You'd > probably have to walk through the lists, comparing the values of all > attributes. Unless you tie the hashes to some class implementing the > necessarry logic in its STORE function? That might be a possibilty... > I agree with you. It makes more sense to modify attributes that are changed instead of appending a new pair which will be discarded in most cases. Is there an easy way to tie a "modified" flag to each attribute that could be changed to indicate a modification. Or use something like a composite key "attribute+operator" with a missing operator defaulting to current behavior. This would allow people to use ":=" when needed and old code should still work correctly. Ken - List info/subscribe/unsubscribe? See http://www.freeradius.org/list/users.html
Re: Change RAD_REPLY item in rlm_perl, not add a new pair
On Tue, Jun 20, 2006 at 11:23:13AM +0300, Boian Jordanov wrote: > On Tuesday 20 June 2006 02:17, Kenneth Marshall wrote: > > Dear Freeradius Users: > > > > I am trying to use rlm_perl to append a number to one > > member of the reply packet using rlm_perl and the %RAD_REPLY > > hash. I am running freeradius-1.1.1. Here is the code that > > I am using, a modified example.pl: > > Edit rlm_perl.c and change T_OP_EQ to T_OP_SET in function get_hv_content > > recomplie and install > Thank you for that information. I am testing this now. Ken Marshall - List info/subscribe/unsubscribe? See http://www.freeradius.org/list/users.html
Re: Change RAD_REPLY item in rlm_perl, not add a new pair
Kenneth Marshall <[EMAIL PROTECTED]> writes: > I am trying to use rlm_perl to append a number to one > member of the reply packet using rlm_perl and the %RAD_REPLY > hash. I am running freeradius-1.1.1. I don't think you can do that with rlm_perl. The inability to specify operator is limiting. rlm_perl will choose T_OP_ADD if the hash value is an array reference. Otherwise it defaults to T_OP_EQ when creating the lists of vps out of the %RAD_REPLY and %RAD_CHECK hashes. There is no way to specify T_OP_SET, which is the functionality you need. I would really love to improve this, but I can't think of any nice way to do it. If one were to add operators to the perl hashes, how could that be done? Adding an additional set of hashes, mapping attribute names to operators? Adding "magic" operator strings to either key or value of the current hashes? I'm afraid that noen of these will be backwards compatible with existing perl scripts using rlm_perl. Another possibility would be to let rlm_perl assume T_OP_SET for modified attributes (unless they are array references). This makes sense to me (and to you it seems, since that's what you expected :-). But I dont't know of an effecient way to implement this. You'd probably have to walk through the lists, comparing the values of all attributes. Unless you tie the hashes to some class implementing the necessarry logic in its STORE function? That might be a possibilty... Bjørn - List info/subscribe/unsubscribe? See http://www.freeradius.org/list/users.html
Re: Change RAD_REPLY item in rlm_perl, not add a new pair
On Tuesday 20 June 2006 02:17, Kenneth Marshall wrote: > Dear Freeradius Users: > > I am trying to use rlm_perl to append a number to one > member of the reply packet using rlm_perl and the %RAD_REPLY > hash. I am running freeradius-1.1.1. Here is the code that > I am using, a modified example.pl: Edit rlm_perl.c and change T_OP_EQ to T_OP_SET in function get_hv_content recomplie and install -- Best Regards, Boian Jordanov SNE Orbitel - Next Generation Telecom tel. +359 2 4004 723 tel. +359 2 4004 002 - List info/subscribe/unsubscribe? See http://www.freeradius.org/list/users.html
