Repeatable problem with Client-Shortname
Here's a strange problem: the Client-Shortname attribute won't match a regexp unless it's inside a string expansion. Simple example: # Fails if (Client-Shortname =~ /./) { update reply { Reply-Message += "XXX" } } # Succeeds if ("%{Client-Shortname}" =~ /./) { update reply { Reply-Message += "YYY" } } (Uncomment "shortname = localhost" from clients.conf). The first test fails to match, but the second one matches. Debug output: ... [files] users: Matched entry steve at line 76 ++[files] returns ok ++[expiration] returns noop ++[logintime] returns noop ++? if (Client-Shortname =~ /./) (Attribute Client-Shortname was not found) ? Evaluating (Client-Shortname =~ /./) -> FALSE ++? if (Client-Shortname =~ /./) -> FALSE ++? if ("%{Client-Shortname}" =~ /./) expand: %{Client-Shortname} -> localhost ? Evaluating ("%{Client-Shortname}" =~ /./) -> TRUE ++? if ("%{Client-Shortname}" =~ /./) -> TRUE ++- entering if ("%{Client-Shortname}" =~ /./) {...} +++[reply] returns noop ++- if ("%{Client-Shortname}" =~ /./) returns noop ++[pap] returns updated Found Auth-Type = PAP ... This is built from the latest code on the v2.1.x branch. If I try with a different request attribute, like Client-IP-Address, it works fine. It's not a big issue, because I can just put the attribute in a string expansion to resolve it, but it was a bit confusing. Thanks, Brian.
Hi,
Here's a strange problem: the Client-Shortname attribute won't match a regexp unless it's inside a string expansion. Simple example:
theres probably a few attributes like that i always use string expansion anyway eg even for "%{User-Name}" - guess thats out of long term habit. alan
Brian Candler wrote:
Here's a strange problem: the Client-Shortname attribute won't match a regexp unless it's inside a string expansion. Simple example:
It isn't a real attribute. You can use real attributes by name. "Fake" attributes like (insert list here), you need to use string expansion. Alan DeKok.
On Thu, Jun 23, 2011 at 06:04:33PM +0200, Alan DeKok wrote:
Brian Candler wrote:
Here's a strange problem: the Client-Shortname attribute won't match a regexp unless it's inside a string expansion. Simple example:
It isn't a real attribute. You can use real attributes by name. "Fake" attributes like (insert list here), you need to use string expansion.
What's the definition of "Fake" here? Client-IP-Address doesn't seem any more real...
Hi,
On Thu, Jun 23, 2011 at 06:04:33PM +0200, Alan DeKok wrote:
Brian Candler wrote:
Here's a strange problem: the Client-Shortname attribute won't match a regexp unless it's inside a string expansion. Simple example:
It isn't a real attribute. You can use real attributes by name. "Fake" attributes like (insert list here), you need to use string expansion.
What's the definition of "Fake" here? Client-IP-Address doesn't seem any more real...
Client-IP- Address is a standard RADIUS attribute.... client shortname is an internal FreeRADIUS attribute... i guess thats what is meant by the above statement (which is why client shortname is not populated from all angles...eg SQL storage of NAS details) alan
On Jun 23, 2011, at 10:20 PM, Alan DeKok wrote:
Brian Candler wrote:
What's the definition of "Fake" here? Client-IP-Address doesn't seem any more real...
If it's in a RADIUS packet, it's real. Others are sometimes real, sometimes not. See the source.
Or, if someone adds documentation, that would be wonderful.
Wouldn't it be wonderful if there was a system where users and developers could both contribute content, quickly and easily, without having to learn GIT, in a format that closely resembles plaintext. Arran Cudbard-Bell a.cudbardb@freeradius.org RADIUS - Half the Size and Complexity of Diameter
On 06/23/2011 09:54 PM, Arran Cudbard-Bell wrote:
If it's in a RADIUS packet, it's real. Others are sometimes real, sometimes not. See the source.
Or, if someone adds documentation, that would be wonderful.
Wouldn't it be wonderful if there was a system where users and developers could both contribute content, quickly and easily, without having to learn GIT, in a format that closely resembles plaintext.
;o) In this particular case, I think it would be more wonderful if we jiggle the FR innards to consistently handle this; it beats a wiki page saying: "This attribute can be used bare here, but this attribute needs to be expanded here". Unless anyone can think of a reason not to, I'll take a look at this tomorrow / next week; it doesn't look terribly hard to solve.
On Thu, Jun 23, 2011 at 10:04:05PM +0100, Phil Mayers wrote:
Unless anyone can think of a reason not to, I'll take a look at this tomorrow / next week; it doesn't look terribly hard to solve.
Since you know this stuff, can you explain why there seem to be two sets of attribute handling code, some in lib/ (e.g. paircmp) and some in main/ (e.g. radius_compare_vps)? This is what has put me off trying to look at this area myself. Thanks, Brian.
On 06/24/2011 09:20 AM, Brian Candler wrote:
On Thu, Jun 23, 2011 at 10:04:05PM +0100, Phil Mayers wrote:
Unless anyone can think of a reason not to, I'll take a look at this tomorrow / next week; it doesn't look terribly hard to solve.
Since you know this stuff, can you explain why there seem to be two sets of attribute handling code, some in lib/ (e.g. paircmp) and some in main/ (e.g. radius_compare_vps)?
I don't know the background, sorry. It's only recently I've looked at the server code code (as opposed to digging into the source for rlm_whatever, which I do a lot to find out how the things actually behave in edge cases). Alan can probably comment. I do know that some of the seemingly duplicate API points do subtly different things. It's probable they could be rationalised, although the resulting API calls might end up with "flags" arguments to control the behaviour, which might be worse, not better. You can argue that putting the behaviour in the function name is a lot more obvious than in a "flags" bitmask.
This is what has put me off trying to look at this area myself.
Start small maybe? Find one pair of calls that look like a duplicate, submit a patch to merge them, repeat/rinse. Assuming it's a problem worth solving, of course. What do we think about the various API doc tool e.g. Doxygen? It might make navigating the source easier for people unfamiliar with it.
Phil Mayers wrote:
I don't know the background, sorry. It's only recently I've looked at the server code code (as opposed to digging into the source for rlm_whatever, which I do a lot to find out how the things actually behave in edge cases). Alan can probably comment.
The stuff in src/lib/ is intended to be used by RADIUS clients and servers. The code in src/main is only for the server. That includes things like callbacks, module registration, etc.
I do know that some of the seemingly duplicate API points do subtly different things. It's probable they could be rationalised, although the resulting API calls might end up with "flags" arguments to control the behaviour, which might be worse, not better. You can argue that putting the behaviour in the function name is a lot more obvious than in a "flags" bitmask.
It's more than that. Clients need basic simple APIs. Servers need complex APIs. Merging the two is hard.
What do we think about the various API doc tool e.g. Doxygen? It might make navigating the source easier for people unfamiliar with it.
That would be wonderful. Alan DeKok.
On 24/06/11 10:53, Alan DeKok wrote:
What do we think about the various API doc tool e.g. Doxygen? It might make navigating the source easier for people unfamiliar with it.
That would be wonderful.
What do people think of this: http://philmayers.github.com/freeradius-server/doxygen/html/ Doxygen source and some files modified with the comment metadata are in this github branch: https://github.com/philmayers/freeradius-server/tree/doxygen Comments welcome. It's very much a proof-of-concept. If people are interested, I used the "libnl" source base as the idea to aim for: http://www.infradead.org/~tgr/libnl/doc-git/api/index.html The libnl source tarball has some neat examples of Doxygen tricks and stuff. tl;dr version of Doxygen: 1. Start each file as follows: /** * @file name.c * @brief One sentence description */ 2. Prefix functions with: /** * @brief does some magic * * This will be turned into a link: process() * This will be a list: * - One * - Two * * @param word the magic word * @return 0 on success, -1 on failure */ int magic(char *word) { } 3. Use a fake meta-source file to build table-of-contents (see doc/source/extra/toc.c)
Phil Mayers wrote:
What do people think of this:
http://philmayers.github.com/freeradius-server/doxygen/html/
Very nice. I've committed the first pass to the main "master" branch.
If people are interested, I used the "libnl" source base as the idea to aim for:
Yes, I'm interested. It looks to be easy to do, and easy to maintain. I tried it with the callgraph options on. It took ~10 min, but all of the functions had caller / callee graphs. The graphs were much more complicated than I expected. Alan DeKok.
On Thu, Jun 23, 2011 at 10:54:38PM +0200, Arran Cudbard-Bell wrote:
Wouldn't it be wonderful if there was a system where users and developers could both contribute content, quickly and easily, without having to learn GIT, in a format that closely resembles plaintext.
markdown is wonderful, especially the [pandoc] variant. There's also a wiki called [gitit] built on pandoc. The backend is git, but users don't know this. It lets you do extremely cool things like downloading wiki pages as PDFs, and viewing wiki pages as slideshows. [pandoc]: http://johnmacfarlane.net/pandoc/ [gitit]: http://gitit.net/ reStructuredText comes a close second. Cheers, Brian.
On Jun 24, 2011, at 10:14 AM, Brian Candler wrote:
On Thu, Jun 23, 2011 at 10:54:38PM +0200, Arran Cudbard-Bell wrote:
Wouldn't it be wonderful if there was a system where users and developers could both contribute content, quickly and easily, without having to learn GIT, in a format that closely resembles plaintext.
markdown is wonderful, especially the [pandoc] variant. There's also a wiki called [gitit] built on pandoc. The backend is git, but users don't know this. It lets you do extremely cool things like downloading wiki pages as PDFs, and viewing wiki pages as slideshows.
[pandoc]: http://johnmacfarlane.net/pandoc/ [gitit]: http://gitit.net/
reStructuredText comes a close second.
Yeah looks like it's a pretty good equivalent to gollum. Unfortunately, clicking the 'history' link on the home page doesn't exactly fill me with confidence that it'll be any less buggy :(. I think a better idea would be to add 'pandoc' support to Gollum. It looks like there's already a wrapper of the Haskell library for this [http://rubygems.org/gems/pandoc-ruby]. Pandoc can also do PDF conversions so it's likely the library responsible Then when people changed the format of a page, gollum could pass the old content through pandoc before writing it out in the new page. This would fix a bunch of issues for us with the crappy mediawiki parser (Wiki Cloth). I'll take a look at it over the weekend if I get time. Or if you know ruby and want to have a crack at it yourself my fork of gollum is here [git://github.com/arr2036/gollum.git]. Just check it out and merge your changes to make sure it works with the FR wiki's file handling stuff (it's slightly different for Mediawiki compatibility). Cheers, Arran
Cheers,
Brian. - List info/subscribe/unsubscribe? See http://www.freeradius.org/list/devel.html
Arran Cudbard-Bell a.cudbardb@freeradius.org RADIUS - Half the complexity of Diameter
I think a better idea would be to add 'pandoc' support to Gollum. It looks like there's already a wrapper of the Haskell library for this [http://rubygems.org/gems/pandoc-ruby]. Pandoc can also do PDF conversions so it's likely the library responsible
... For PDF support in gitit. Arran Cudbard-Bell a.cudbardb@freeradius.org RADIUS - Half the complexity of Diameter
On Fri, Jun 24, 2011 at 10:44:02AM +0200, Arran Cudbard-Bell wrote:
Yeah looks like it's a pretty good equivalent to gollum. Unfortunately, clicking the 'history' link on the home page doesn't exactly fill me with confidence that it'll be any less buggy :(.
Hmm, hadn't noticed that! History works on my local test installation. I'll raise it on the gitit group.
On Fri, Jun 24, 2011 at 11:14:29AM +0100, Brian Candler wrote:
On Fri, Jun 24, 2011 at 10:44:02AM +0200, Arran Cudbard-Bell wrote:
Yeah looks like it's a pretty good equivalent to gollum. Unfortunately, clicking the 'history' link on the home page doesn't exactly fill me with confidence that it'll be any less buggy :(.
Hmm, hadn't noticed that! History works on my local test installation. I'll raise it on the gitit group.
OK, the problem has been found and fixed: http://groups.google.com/group/gitit-discuss/msg/8e519d464ab9a76b Basically, the history page was only looking for changes in the last year, and was aborting if there weren't any. It has been changed to display the last 100 changes, with a continuation link. But it would only have affected stale pages :-) I also received a private reply from someone who said they had used gitit heavily in production at their organisation (a childrens' hospital) for the past 18 months, and it had been rock solid. Regards, Brian.
On Jun 25, 2011, at 11:14 AM, Brian Candler wrote:
On Fri, Jun 24, 2011 at 11:14:29AM +0100, Brian Candler wrote:
On Fri, Jun 24, 2011 at 10:44:02AM +0200, Arran Cudbard-Bell wrote:
Yeah looks like it's a pretty good equivalent to gollum. Unfortunately, clicking the 'history' link on the home page doesn't exactly fill me with confidence that it'll be any less buggy :(.
Hmm, hadn't noticed that! History works on my local test installation. I'll raise it on the gitit group.
OK, the problem has been found and fixed: http://groups.google.com/group/gitit-discuss/msg/8e519d464ab9a76b
Basically, the history page was only looking for changes in the last year, and was aborting if there weren't any. It has been changed to display the last 100 changes, with a continuation link. But it would only have affected stale pages :-)
Aha, ok that makes sense :)
I also received a private reply from someone who said they had used gitit heavily in production at their organisation (a childrens' hospital) for the past 18 months, and it had been rock solid.
While that may be, a Childrens' Hospital isn't likely to be exercising all the features of the wiki (expressions, code highlighting etc..). I don't think i've ever used an open source product which hasn't had at least one minor bug, so the ability to write patches to fix issues is a big bonus. I don't have time to learn Haskell, on top of Perl (which I need for my job) and French (which I need for my girlfriend (acquisition), and I guess ordering beer.... maybe food?..). If you can convince one of the more permanent list members ( Alex Clouter, Phil Mayers, Peter Nixon, One of the Alans etc..), that a) the current wiki needs replacing and b) that they really want to learn Haskell, then we'd consider switching gitit. Cheers, Arran Arran Cudbard-Bell a.cudbardb@freeradius.org RADIUS - Half the complexity of Diameter
On Sat, Jun 25, 2011 at 12:20:06PM +0200, Arran Cudbard-Bell wrote:
While that may be, a Childrens' Hospital isn't likely to be exercising all the features of the wiki (expressions, code highlighting etc..).
I don't think i've ever used an open source product which hasn't had at least one minor bug, so the ability to write patches to fix issues is a big bonus.
I don't know Haskell either, but I've found the mailing list (and in particular the author) to be responsive and helpful. And the fact that Haskell is a functional language, and compiles into a binary executable, suggests to me that the finished product will be very robust. I'm not advocating any change for change's sake. I think it's a worthwhile option for anyone looking to deploy a new wiki. I cringe every time I have to use yet another different wiki markup; to me, markdown is the least annoying of them all. But if you're using gollum, and happy with it, then github flavoured markdown is probably good enough. Regards, Brian.
I cringe every time I have to use yet another different wiki markup; to me, markdown is the least annoying of them all.
Yeah I like it too, it's almost a shame that FreeRADIUS has standardised on RST, but I guess the 'official' standard for RST is more featureful than Markdown. Standards based Markdown doesn't even include support for tables, even though this is defined by unofficial extensions.
But if you're using gollum, and happy with it, then github flavoured markdown is probably good enough.
It's actually passed through to a C library called 'upskirt', via 'red carpet' (or should that be 'red carpet' via 'upskirt' *sigh* those ruby developers...) Anyway, upskirt implements pretty much all the unofficial extensions to Markdown, so if anything, markdown will work *better* in GitHub and Gollum than anywhere else. The possibility of pan-doc support is pretty exciting though, it'd allow you to wikitise existing HTML pages from offsite, and convert pretty much any wiki language (so long as the rendered could output HTML) to RST or Markdown. No time this past weekend but definitely a priority. Cheers, Arran Arran Cudbard-Bell a.cudbardb@freeradius.org RADIUS - Half the complexity of Diameter
On 06/23/2011 07:25 PM, Brian Candler wrote:
On Thu, Jun 23, 2011 at 06:04:33PM +0200, Alan DeKok wrote:
Brian Candler wrote:
Here's a strange problem: the Client-Shortname attribute won't match a regexp unless it's inside a string expansion. Simple example:
It isn't a real attribute. You can use real attributes by name. "Fake" attributes like (insert list here), you need to use string expansion.
What's the definition of "Fake" here? Client-IP-Address doesn't seem any more real...
The difference is that "%{Client-Shortname}" runs through xlat.c and eventually xlat_packet, which "knows" about magic attributes. "if (Client-Shortname == ...)" runs through modcall.c and evaluate.c, ending up in radius_do_cmp. This doesn't know about special attributes; only real ones, or anything with a paircompare() registered e.g. LDAP-Group. How, then, does Client-IP-Address work? Ah - rlm_expr registers a paircompare() for this! It is confusing.
participants (5)
-
Alan Buxey -
Alan DeKok -
Arran Cudbard-Bell -
Brian Candler -
Phil Mayers