[master] questions about recent changes in xlat

classic Classic list List threaded Threaded
13 messages Options
| Threaded
Open this post in threaded view
|

[master] questions about recent changes in xlat

Devel mailing list
Hello,


A couple of recent changes I've noticed in FreeRADIUS xlat framework:

1)
"%%" is no longer transformed to "%" (it remains %%).
Function xlat_eval_one_letter has a case for '%' but it seems it's not called anymore.
In function xlat_sync_eval which should call xlat_eval_one_letter, I see that node->type is 1 (XLAT_LITERAL) instead of 2 (XLAT_ONE_LETTER).

Is this change intended ?

According to the documentation it should work:
https://github.com/FreeRADIUS/freeradius-server/blob/master/doc/antora/modules/unlang/pages/xlat/character.adoc


2)
Using a form of xlat functions with no parameter is no longer working.
For example: %{myxlat}

This used to work (not too long ago).
This is useful to me, although I can still get the same result with: %{myxlat:}
But this is a bit ugly though...

Do you think we could get this to work again without the colon ?



Regards,
Nicolas.

This message contains information that may be privileged or confidential and is the property of the Capgemini Group. It is intended only for the person to whom it is addressed. If you are not the intended recipient, you are not authorized to read, print, retain, copy, disseminate, distribute, or use this message or any part thereof. If you receive this message in error, please notify the sender immediately and delete all copies of this message.
-
List info/subscribe/unsubscribe? See http://www.freeradius.org/list/devel.html
| Threaded
Open this post in threaded view
|

Re: [master] questions about recent changes in xlat

Alan DeKok-2
On Oct 24, 2020, at 8:44 AM, Chaigneau, Nicolas via Freeradius-Devel <[hidden email]> wrote:
> A couple of recent changes I've noticed in FreeRADIUS xlat framework:
>
> 1)
> "%%" is no longer transformed to "%" (it remains %%).
> Function xlat_eval_one_letter has a case for '%' but it seems it's not called anymore.
> In function xlat_sync_eval which should call xlat_eval_one_letter, I see that node->type is 1 (XLAT_LITERAL) instead of 2 (XLAT_ONE_LETTER).
>
> Is this change intended ?

  Yes.  We've updated the parser to be smarter.  It no longer need %%.  Which was always ugly.

> According to the documentation it should work:
> https://github.com/FreeRADIUS/freeradius-server/blob/master/doc/antora/modules/unlang/pages/xlat/character.adoc

  We'll update the documentation, and also the upgrade guide.

> 2)
> Using a form of xlat functions with no parameter is no longer working.
> For example: %{myxlat}
>
> This used to work (not too long ago).
> This is useful to me, although I can still get the same result with: %{myxlat:}
> But this is a bit ugly though...
>
> Do you think we could get this to work again without the colon ?

  Possibly.  We're trying to regularize all of the parsing, expansions, etc.  Which means that some changes creep in.

  The main issue with not using the ":" is that the syntax becomes ambiguous.  Is %{foo} an attribute reference, or an xlat function?  You can't tell.

  Alan DeKok.


-
List info/subscribe/unsubscribe? See http://www.freeradius.org/list/devel.html
| Threaded
Open this post in threaded view
|

Re: [master] questions about recent changes in xlat

arr2036


> On Oct 24, 2020, at 8:30 AM, Alan DeKok <[hidden email]> wrote:
>
> On Oct 24, 2020, at 8:44 AM, Chaigneau, Nicolas via Freeradius-Devel <[hidden email]> wrote:
>> A couple of recent changes I've noticed in FreeRADIUS xlat framework:
>>
>> 1)
>> "%%" is no longer transformed to "%" (it remains %%).
>> Function xlat_eval_one_letter has a case for '%' but it seems it's not called anymore.
>> In function xlat_sync_eval which should call xlat_eval_one_letter, I see that node->type is 1 (XLAT_LITERAL) instead of 2 (XLAT_ONE_LETTER).
>>
>> Is this change intended ?
>
>  Yes.  We've updated the parser to be smarter.  It no longer need %%.  Which was always ugly.

The new escape sequence is \% which is far more intuitive.

>> According to the documentation it should work:
>> https://github.com/FreeRADIUS/freeradius-server/blob/master/doc/antora/modules/unlang/pages/xlat/character.adoc
>
>  We'll update the documentation, and also the upgrade guide.
>
>> 2)
>> Using a form of xlat functions with no parameter is no longer working.
>> For example: %{myxlat}
>>
>> This used to work (not too long ago).
>> This is useful to me, although I can still get the same result with: %{myxlat:}
>> But this is a bit ugly though...
>>
>> Do you think we could get this to work again without the colon ?

No.

>  Possibly.  We're trying to regularize all of the parsing, expansions, etc.  Which means that some changes creep in.
>
>  The main issue with not using the ":" is that the syntax becomes ambiguous.  Is %{foo} an attribute reference, or an xlat function?  You can't tell.

Exactly.  The old xlat code didn't support multi-pass resolution of attributes or functions because it was impossible to tell what was an attribute and what was a function.

That's why ':' is no longer allowed as a list specifier for attribute references in xlats.

-Arran
-
List info/subscribe/unsubscribe? See http://www.freeradius.org/list/devel.html
| Threaded
Open this post in threaded view
|

RE: [master] questions about recent changes in xlat

Devel mailing list
>
> >  Yes.  We've updated the parser to be smarter.  It no longer need %%.  Which was always ugly.
>
> The new escape sequence is \% which is far more intuitive.



Thanks, but... I can't figure out how to handle escaping of a "%" right now. :/
I've tried the following:

1)
\%{
This returns an error: "Missing closing brace".

2)
\%D
Xlat outputs: "\20201024".

So... I'm confused right now. What am I doing wrong ?



> >> Do you think we could get this to work again without the colon ?
>
> No.
>
> >  Possibly.  We're trying to regularize all of the parsing, expansions, etc.  Which means that some changes creep in.
> >
> >  The main issue with not using the ":" is that the syntax becomes ambiguous.  Is %{foo} an attribute reference, or an xlat function?  You can't tell.
>
> Exactly.  The old xlat code didn't support multi-pass resolution of attributes or functions because it was impossible to tell what was an attribute and what was a function.
>
> That's why ':' is no longer allowed as a list specifier for attribute references in xlats.


Thanks for the explanation. I'll adapt my functions then. :)




This message contains information that may be privileged or confidential and is the property of the Capgemini Group. It is intended only for the person to whom it is addressed. If you are not the intended recipient, you are not authorized to read, print, retain, copy, disseminate, distribute, or use this message or any part thereof. If you receive this message in error, please notify the sender immediately and delete all copies of this message.

-
List info/subscribe/unsubscribe? See http://www.freeradius.org/list/devel.html
| Threaded
Open this post in threaded view
|

Re: [master] questions about recent changes in xlat

Alan DeKok-2
On Oct 24, 2020, at 6:00 PM, Chaigneau, Nicolas via Freeradius-Devel <[hidden email]> wrote:

> Thanks, but... I can't figure out how to handle escaping of a "%" right now. :/
> I've tried the following:
>
> 1)
> \%{
> This returns an error: "Missing closing brace".
>
> 2)
> \%D
> Xlat outputs: "\20201024".
>
> So... I'm confused right now. What am I doing wrong ?

  Not much.

  I've added some tests in commit 95f1d5c1.

  And yes, \%{ results in \%{.

  Alan DeKok.


-
List info/subscribe/unsubscribe? See http://www.freeradius.org/list/devel.html
| Threaded
Open this post in threaded view
|

Re: [master] questions about recent changes in xlat

Alan DeKok-2
In reply to this post by Devel mailing list
On Oct 24, 2020, at 6:00 PM, Chaigneau, Nicolas via Freeradius-Devel <[hidden email]> wrote

> Thanks, but... I can't figure out how to handle escaping of a "%" right now. :/
> I've tried the following:
>
> 1)
> \%{
> This returns an error: "Missing closing brace".
>
> 2)
> \%D
> Xlat outputs: "\20201024".
>
> So... I'm confused right now. What am I doing wrong ?

  Do you have more context for these examples?  i.e. larger xlat strings you're using.

  Alan DeKok.


-
List info/subscribe/unsubscribe? See http://www.freeradius.org/list/devel.html
| Threaded
Open this post in threaded view
|

RE: [master] questions about recent changes in xlat

Devel mailing list
In reply to this post by Alan DeKok-2

> >
> > So... I'm confused right now. What am I doing wrong ?
>
>   Not much.
>
>   I've added some tests in commit 95f1d5c1.
>
>   And yes, \%{ results in \%{.



Alright, I found out why it does not work.

In function _xlat_eval:

        len = xlat_tokenize_ephemeral(ctx, &node, NULL,
                                      &FR_SBUFF_IN(fmt, strlen(fmt)),
                                      NULL, &(tmpl_rules_t){ .dict_def = request->dict });


_xlat_eval does not provide a "fr_sbuff_parse_rules_t" with the escape function.
As compared with the code in unit tests, which does:

                        fr_sbuff_parse_rules_t p_rules = { .escapes = &fr_value_unescape_double };

                        slen = xlat_tokenize_ephemeral(xlat_ctx, &head, NULL,
                                                       &FR_SBUFF_IN(fmt, talloc_array_length(fmt) - 1), &p_rules, NULL);



This message contains information that may be privileged or confidential and is the property of the Capgemini Group. It is intended only for the person to whom it is addressed. If you are not the intended recipient, you are not authorized to read, print, retain, copy, disseminate, distribute, or use this message or any part thereof. If you receive this message in error, please notify the sender immediately and delete all copies of this message.

-
List info/subscribe/unsubscribe? See http://www.freeradius.org/list/devel.html
| Threaded
Open this post in threaded view
|

Re: [master] questions about recent changes in xlat

Alan DeKok-2
On Oct 25, 2020, at 12:38 PM, Chaigneau, Nicolas via Freeradius-Devel <[hidden email]> wrote

> Alright, I found out why it does not work.
>
> In function _xlat_eval:
>
> len = xlat_tokenize_ephemeral(ctx, &node, NULL,
>      &FR_SBUFF_IN(fmt, strlen(fmt)),
>      NULL, &(tmpl_rules_t){ .dict_def = request->dict });
>
>
> _xlat_eval does not provide a "fr_sbuff_parse_rules_t" with the escape function.
> As compared with the code in unit tests, which does:
>
> fr_sbuff_parse_rules_t p_rules = { .escapes = &fr_value_unescape_double };
>
> slen = xlat_tokenize_ephemeral(xlat_ctx, &head, NULL,
>       &FR_SBUFF_IN(fmt, talloc_array_length(fmt) - 1), &p_rules, NULL);

  Changing that doesn't make any difference in my tests.

  I've added a test to src/tests/keywords/xlat-escape.  It does what I expect it to do.

  So... what's the use-case for your tests?  How can I reproduce them here?

  Alan DeKok.


-
List info/subscribe/unsubscribe? See http://www.freeradius.org/list/devel.html
| Threaded
Open this post in threaded view
|

RE: [master] questions about recent changes in xlat

Devel mailing list
> > Alright, I found out why it does not work.
> >
> > In function _xlat_eval:
> >
> > len = xlat_tokenize_ephemeral(ctx, &node, NULL,
> >      &FR_SBUFF_IN(fmt, strlen(fmt)),
> >      NULL, &(tmpl_rules_t){ .dict_def = request->dict });
> >
> >
> > _xlat_eval does not provide a "fr_sbuff_parse_rules_t" with the escape function.
> > As compared with the code in unit tests, which does:
> >
> > fr_sbuff_parse_rules_t p_rules = { .escapes = &fr_value_unescape_double };
> >
> > slen = xlat_tokenize_ephemeral(xlat_ctx, &head, NULL,
> >       &FR_SBUFF_IN(fmt, talloc_array_length(fmt) - 1), &p_rules, NULL);
>
>   Changing that doesn't make any difference in my tests.
>
>   I've added a test to src/tests/keywords/xlat-escape.  It does what I expect it to do.
>
>   So... what's the use-case for your tests?  How can I reproduce them here?



My use case is the following:

I receive a string buffer from an external source, which can contain any text (encoding doesn't matter).
Xlat expressions are allowed in this buffer.
This entails that if the text contains some random "%" characters which would trigger expansion, they must be escaped.

For example: "this must be xlated: %D, these however must not be modified: \%{  \%D  \x61 \t".

Previously I just needed to double the "%" ("%%") to make it work.
Now as I understand, I have to change the escaping (in the received text) as follows: "\%".

This does not work with "xlat_eval" function, because it does not provide an unescape function to xlat_tokenize_ephemeral  (through parameter "fr_sbuff_parse_rules_t const *p_rules")


So I need to call xlat_tokenize_ephemeral with my own sbuff unescape rules, that I defined as follow:

fr_sbuff_unescape_rules_t fr_value_unescape_xlat = {
        .name = "xlat",
        .chr = '\\',
        .subs = {
                ['%'] = '%', /* xlat expansions */
        },
        .do_hex = false,
        .do_oct = false
};

(I can't use fr_value_unescape_double because this would do things I don't want, e.g. "\x61" would be modified to "a").


The following code shows how to reproduce my issue (the two "p_rules" commented out do not provide the result that  I want, the uncommented "p_rules" does).

        char *fmt = "this must be xlated: %D, these however must not be modified: \\%{  \\%D  \\x61 \\t";
        ssize_t slen;
        TALLOC_CTX *xlat_ctx = talloc_init_const("xlat");
        xlat_exp_t *head = NULL;
        //fr_sbuff_parse_rules_t p_rules = { }; // nope
        //fr_sbuff_parse_rules_t p_rules = { .escapes = &fr_value_unescape_double }; // nope
        fr_sbuff_parse_rules_t p_rules = { .escapes = &fr_value_unescape_xlat };

        slen = xlat_tokenize_ephemeral(xlat_ctx, &head, NULL,
                                       &FR_SBUFF_IN(fmt, strlen(fmt)), &p_rules, NULL);

        slen = xlat_eval_compiled(out, outlen, request, head, NULL, NULL);





This message contains information that may be privileged or confidential and is the property of the Capgemini Group. It is intended only for the person to whom it is addressed. If you are not the intended recipient, you are not authorized to read, print, retain, copy, disseminate, distribute, or use this message or any part thereof. If you receive this message in error, please notify the sender immediately and delete all copies of this message.

-
List info/subscribe/unsubscribe? See http://www.freeradius.org/list/devel.html
| Threaded
Open this post in threaded view
|

Re: [master] questions about recent changes in xlat

Alan DeKok-2
On Oct 25, 2020, at 2:59 PM, Chaigneau, Nicolas via Freeradius-Devel <[hidden email]> wrote:
> My use case is the following:
>
> I receive a string buffer from an external source, which can contain any text (encoding doesn't matter).
> Xlat expressions are allowed in this buffer.
> This entails that if the text contains some random "%" characters which would trigger expansion, they must be escaped.
>
> For example: "this must be xlated: %D, these however must not be modified: \%{  \%D  \x61 \t".

  That's an issue.  You want *some* things to be escaped, but (sort of randomly) other things to *not* be escaped.

  One of the things we've been doing in v4 is to regularize all of the escaping.  We're gradually getting rid of "ad hoc" things like the above, and just moving to:

* strings are 'single quoted' - minimal escaping, no xlat expansions
* strings are "double quoted" - more escaping, and xlat expansions.

  What you're asking for is something else.  The escaping rules will be custom...

  So the best way is for you to just use xlat_tokenize_ephemeral(), which lets you define your own escaping rules.  Because the rules you propose for this case aren't generalizable to other cases.  You can then call xlat_eval_compiled() to get the results.

  i.e. just re-implement xlat_eval(), but with your rules.

  We've been gradually moving all of the "bare xlat expansion" calls to using tmpls, which contain the escaping rules.  This change avoids the issue you're running into, by making the rules simple:  "It's a double quoted string".

  There are still a number of calls to xlat_eval() in the server.  e.g. creating the detail filename.  These calls will have exactly the same issue you're seeing.  So they're broken to some extent.  But since people don't generally put "%" into filenames, it's less of an issue.

  We're working on fixing all that.  But there's only so much time in a day.  :(

  Alan DeKok.


-
List info/subscribe/unsubscribe? See http://www.freeradius.org/list/devel.html
| Threaded
Open this post in threaded view
|

RE: [master] questions about recent changes in xlat

Devel mailing list
Thanks for your detailled explanation.
I was a bit confused when you and Arran told me escaping "just worked", but of course you couldn't guess what I was trying to do (custom things using some FreeRADIUS functions that are not meant for that).

So I'll just use my own version of xlat_eval as you suggested.
By the way, the new parsing facilities you've added recently makes this very easy. I like that :)

Keep up the good work!


Regards,
Nicolas.


>   That's an issue.  You want *some* things to be escaped, but (sort of randomly) other things to *not* be escaped.
>
>   One of the things we've been doing in v4 is to regularize all of the escaping.  We're gradually getting rid of "ad hoc" things like the above, and just moving to:
>
> * strings are 'single quoted' - minimal escaping, no xlat expansions
> * strings are "double quoted" - more escaping, and xlat expansions.
>
>   What you're asking for is something else.  The escaping rules will be custom...
>
>   So the best way is for you to just use xlat_tokenize_ephemeral(), which lets you define your own escaping rules.  Because the rules you propose for this case aren't generalizable to other cases.  You can then call xlat_eval_compiled() to get the results.
>
>   i.e. just re-implement xlat_eval(), but with your rules.
>
>   We've been gradually moving all of the "bare xlat expansion" calls to using tmpls, which contain the escaping rules.  This change avoids the issue you're running into, by making the rules simple:  "It's a double quoted string".
>
>   There are still a number of calls to xlat_eval() in the server.  e.g. creating the detail filename.  These calls will have exactly the same issue you're seeing.  So they're broken to some extent.  But since people don't generally put "%" into filenames, it's less of an issue.
>
>   We're working on fixing all that.  But there's only so much time in a day.  :(
This message contains information that may be privileged or confidential and is the property of the Capgemini Group. It is intended only for the person to whom it is addressed. If you are not the intended recipient, you are not authorized to read, print, retain, copy, disseminate, distribute, or use this message or any part thereof. If you receive this message in error, please notify the sender immediately and delete all copies of this message.

-
List info/subscribe/unsubscribe? See http://www.freeradius.org/list/devel.html
| Threaded
Open this post in threaded view
|

Re: [master] questions about recent changes in xlat

Alan DeKok-2
On Oct 26, 2020, at 2:11 PM, Chaigneau, Nicolas via Freeradius-Devel <[hidden email]> wrote:
>
> Thanks for your detailled explanation.
> I was a bit confused when you and Arran told me escaping "just worked", but of course you couldn't guess what I was trying to do (custom things using some FreeRADIUS functions that are not meant for that).

  They're many for escaping, just for double / single-quoted strings.

> So I'll just use my own version of xlat_eval as you suggested.
> By the way, the new parsing facilities you've added recently makes this very easy. I like that :)

  That's Arran's design.  Very nice!

> Keep up the good work!

  Thanks.  We'll get v4 out sometime soon, I hope.

  Alan DeKok.


-
List info/subscribe/unsubscribe? See http://www.freeradius.org/list/devel.html
| Threaded
Open this post in threaded view
|

Re: [master] questions about recent changes in xlat

arr2036
In reply to this post by Devel mailing list


> On 26 Oct 2020, at 12:11, Chaigneau, Nicolas via Freeradius-Devel <[hidden email]> wrote:
>
> Thanks for your detailled explanation.
> I was a bit confused when you and Arran told me escaping "just worked", but of course you couldn't guess what I was trying to do (custom things using some FreeRADIUS functions that are not meant for that).

Yes.

> So I'll just use my own version of xlat_eval as you suggested.
> By the way, the new parsing facilities you've added recently makes this very easy. I like that :)

Thanks :) It’s usually 3:1 reduction in the overall amount of code when we convert things to sbuffs... and yes making your own custom escaping schemes much easier.  

One of the things that’s important to understand is how the rules get applied.  The parse rules passed into the xlat tokenizer only change how the string outside the expansions gets unescaped.  with “foo %{bar: args} baz”, parsing of args is not effected by the rules passed in.  You can think of the string being parsed more like “foo “%{bar: args}” baz”.

This means we can have “%{sql:select * from bar where foo = “bar”}” without escaping the double quotes.  This works because in the grammar applied to the arguments of the sql function “ isn’t a terminal sequence... which is both awesome because you don’t need to escape double quotes, and a bit weird.
 
-Arran

>
> Keep up the good work!
>
>
> Regards,
> Nicolas.
>
>
>>  That's an issue.  You want *some* things to be escaped, but (sort of randomly) other things to *not* be escaped.
>>
>>  One of the things we've been doing in v4 is to regularize all of the escaping.  We're gradually getting rid of "ad hoc" things like the above, and just moving to:
>>
>> * strings are 'single quoted' - minimal escaping, no xlat expansions
>> * strings are "double quoted" - more escaping, and xlat expansions.
>>
>>  What you're asking for is something else.  The escaping rules will be custom...
>>
>>  So the best way is for you to just use xlat_tokenize_ephemeral(), which lets you define your own escaping rules.  Because the rules you propose for this case aren't generalizable to other cases.  You can then call xlat_eval_compiled() to get the results.
>>
>>  i.e. just re-implement xlat_eval(), but with your rules.
>>
>>  We've been gradually moving all of the "bare xlat expansion" calls to using tmpls, which contain the escaping rules.  This change avoids the issue you're running into, by making the rules simple:  "It's a double quoted string".
>>
>>  There are still a number of calls to xlat_eval() in the server.  e.g. creating the detail filename.  These calls will have exactly the same issue you're seeing.  So they're broken to some extent.  But since people don't generally put "%" into filenames, it's less of an issue.
>>
>>  We're working on fixing all that.  But there's only so much time in a day.  :(
> This message contains information that may be privileged or confidential and is the property of the Capgemini Group. It is intended only for the person to whom it is addressed. If you are not the intended recipient, you are not authorized to read, print, retain, copy, disseminate, distribute, or use this message or any part thereof. If you receive this message in error, please notify the sender immediately and delete all copies of this message.
>
> -
> List info/subscribe/unsubscribe? See http://www.freeradius.org/list/devel.html


-
List info/subscribe/unsubscribe? See http://www.freeradius.org/list/devel.html