From: Alan T. DeKok Date: Tue, 5 Sep 2023 17:35:20 +0000 (-0400) Subject: mark up "@todo" with more comments and classifications X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=e3cd70e0dd1063a147fa1e33cff5849c7902ac67;p=thirdparty%2Ffreeradius-server.git mark up "@todo" with more comments and classifications --- diff --git a/src/tests/unit/condition/base.txt b/src/tests/unit/condition/base.txt index f12f9189d26..1a9e6d0b29d 100644 --- a/src/tests/unit/condition/base.txt +++ b/src/tests/unit/condition/base.txt @@ -34,13 +34,13 @@ condition (ok\ foo || handled) match ERROR offset 4: Unexpected text after enum value. Expected operator # -# @todo - This is just a bare word comparison? +# @todo - parsing - This is just a bare word comparison, and should be disallowed? # condition (Service-Type == 000-111) match (Service-Type == (0 - 111)) # -# @todo - many of these "invalid operator" errors should have additional code added to xlat_expr.c, +# @todo - parsing - many of these "invalid operator" errors should have additional code added to xlat_expr.c, # so that it finds out which specific parse problem it is, and gives a better error message. # condition (ok FOO handled) @@ -76,14 +76,11 @@ match !%{rcode:'ok'} condition !(ok) match !%{rcode:'ok'} -# -# @todo - negation is invalid -# condition !!ok match ERROR offset 2: Double operator is invalid # -# @todo - do peephole optimization to get rid of double negation? +# @todo - peephole - do optimization to get rid of double negation? # condition !(!ok) match !!%{rcode:'ok'} @@ -108,7 +105,7 @@ condition (!(&User-Name == &User-Password) || (&Filter-Id == &Reply-Message)) match (!(&User-Name == &User-Password) || (&Filter-Id == &Reply-Message)) # -# @todo - this is really for unit_test_attribute to check? Did we parse the whole thing? +# @todo - parsing - unit_test_attribute should really check if we read the whole string # condition ((a == b) || (c == d))) match ((a == b) || (c == d)) @@ -134,10 +131,6 @@ match ERROR offset 1: Cannot use list references in condition condition "hello" == &reply match ERROR offset 12: Cannot use list references in condition - -# -# @todo - new expressions do not do normalization or peephole optimizations -# condition &User-Name == &User-Password match (&User-Name == &User-Password) @@ -211,7 +204,7 @@ condition &User-Name == 'bob' match (&User-Name == 'bob') # -# @todo - this should probably be a parse error, as there is no VALUE named 'bob' +# @todo - parsing - bare-word bob isn't a valid enum name # condition &User-Name == bob match (&User-Name == bob) @@ -221,7 +214,7 @@ condition &Session-Timeout == 10 match (&Session-Timeout == 10) # -# @todo - no automatic type casting +# @todo - parsing - no automatic type casting # condition &Session-Timeout == '10' match (&Session-Timeout == '10') @@ -240,7 +233,7 @@ condition X == 1 match ERROR offset 10: Failed parsing string as type 'uint32' # -# @todo - this should probably be a parse error, as there is no VALUE named 'X' +# @todo - parsing - there is no enum named "X" # condition &NAS-Port == X match (&NAS-Port == X) @@ -436,9 +429,6 @@ match (%(cast:uint32 "%{md4: 1 + 1}") < &NAS-Port) # The string gets parsed as an IP address. # -# -# @todo - maybe the cast is suppressed? or... TBD later -# condition &Filter-Id == &Framed-IP-Address match (&Filter-Id == &Framed-IP-Address) @@ -512,7 +502,7 @@ match (&Acct-Input-Octets-64 > %(cast:string "%{Session-Timeout}")) # # Parse OIDs into known attributes, where possible. # -# @todo - Perhaps this isn't done because the xlat name is taken from the +# @todo - peephole - Perhaps this isn't done because the xlat name is taken from the # input, and not from the canonicalized names. # condition &26.24757.84.9.5.4 == 0x1a99 @@ -554,7 +544,7 @@ match ERROR offset 12: Invalid array index '-1' (should be between 0-1000) # # Sometimes the attribute/condition parser needs to fallback to bare words # -# @todo - this is likely treating the LHS as an enum which is impossible here +# @todo - bare word - this is likely treating the LHS as an enum which is impossible here # condition request.Foo == 'request.Foo' match (request.Foo == 'request.Foo') @@ -568,7 +558,7 @@ match ((&request.Foo + Bar) == 'request.Foo+Bar') #match ERROR offset 13: Unexpected text after attribute reference # -# @todo - this is wrong +# @todo - bare word - this is wrong # condition 'request.Foo+d' == &request.Foo+Bar match ('request.Foo+d' == (&request.Foo + Bar)) @@ -595,7 +585,7 @@ match false # 'Unknown' attributes which are defined in the main dictionary # should be resolved to their real names. # -# @todo - nope +# @todo - peephole - resolve it and change its name # condition &1 == 0x616263 match (&1 == 0x616263) @@ -644,18 +634,6 @@ match (192.168.0.0/24 > &NAS-IP-Address) condition (&reply.) match &reply. -# -# Expansions of environment variables -# and empty strings -# -# @todo - disabled due to moving cf_expand_variables() from -# the condition parser to cf_file.c. The new xlat expression parser -# will *not* optimize this at parse time, but *will* optimize -# this at purification time. -# -#condition ("$ENV{SOMETHING_OR_OTHER}" == '') -#match true - # # Attributes with a protocol namespace # @@ -671,7 +649,7 @@ match false # # More short-circuit evaluations # -# @todo - optimise at parse time +# @todo - peephole - optimise at parse time # condition (&User-Name == "bob") && (false) match ((&User-Name == "bob") && false) diff --git a/src/tests/unit/condition/filter.txt b/src/tests/unit/condition/filter.txt index 6b95cb6f250..5b81be1de79 100644 --- a/src/tests/unit/condition/filter.txt +++ b/src/tests/unit/condition/filter.txt @@ -32,7 +32,8 @@ match ERROR offset 12: Invalid array index '-1' (should be between 0-1000) # conditional filters. # # And we do not (yet) allow for filters on leaf attributes. See -# @todo in tmpl_attr_parse_filter(() +# +# @todo - feature - allowe expressions in tmpl_attr_parse_filter(() # condition &User-Name[&User-Name == 'bar'] == "foo" match ERROR offset 12: Invalid filter - cannot use filter on leaf attributes @@ -48,7 +49,7 @@ match ERROR offset 12: Invalid filter - cannot use filter on leaf attributes # # which is a loop that returns &TLS-Certificate. # -# @todo - this error is misleading and wrong. +# @todo - feature - this error is misleading and wrong. # condition &TLS-Certificate[&Common-Name == 'user@example.com'] == bar match ERROR offset 1: Cannot use list references in condition diff --git a/src/tests/unit/file.txt b/src/tests/unit/file.txt index 35a81da1aa5..b7eac60ecb7 100644 --- a/src/tests/unit/file.txt +++ b/src/tests/unit/file.txt @@ -9,7 +9,7 @@ proto-dictionary radius # # Fully specified paths. # -# @todo - we arguably want to force nesting on these attributes? Or change the nesting when printed? +# @todo - future - we arguably want to force nesting on these attributes? Or change the nesting when printed? # read_file files/cisco_avpair.txt match User-Name = "bob", User-Password = "hello", Vendor-Specific.Cisco.AVPair = "1", Vendor-Specific.Cisco.AVPair += "2", Vendor-Specific.Cisco.AVPair += "3", Vendor-Specific.Cisco.AVPair += "4" diff --git a/src/tests/unit/protocols/radius/enum.txt b/src/tests/unit/protocols/radius/enum.txt index 22c6759cf12..eb6708d2c04 100644 --- a/src/tests/unit/protocols/radius/enum.txt +++ b/src/tests/unit/protocols/radius/enum.txt @@ -11,9 +11,6 @@ fuzzer-out radius encode-pair Unit-TLV = { Test-Enum-Integer64 = one } match fe 0c 0c 0a 00 00 00 00 00 00 00 01 -# -# @todo - find out why this isn't an enum lookup :( -# decode-pair - match Unit-TLV = { Test-Enum-Integer64 = one } diff --git a/src/tests/unit/protocols/radius/time_delta.txt b/src/tests/unit/protocols/radius/time_delta.txt index 0887cc4570b..dcf3a843dbc 100644 --- a/src/tests/unit/protocols/radius/time_delta.txt +++ b/src/tests/unit/protocols/radius/time_delta.txt @@ -17,9 +17,6 @@ match fe 08 02 06 00 00 00 05 decode-pair - match Unit-TLV = { Delta-MSec = 5 } -# -# @todo - not yet converted to make nested TLVs! -# pair Unit-TLV.Delta-Sec = 10 match Unit-TLV.Delta-Sec = 10 diff --git a/src/tests/unit/protocols/radius/vendor.txt b/src/tests/unit/protocols/radius/vendor.txt index 957c68bee37..2e780a69db0 100644 --- a/src/tests/unit/protocols/radius/vendor.txt +++ b/src/tests/unit/protocols/radius/vendor.txt @@ -121,18 +121,12 @@ match Vendor-Specific.6809.1 = 0xabcdef encode-pair 26.6809.1.2 = 0xabcdef match 1a 0d 00 00 1a 99 01 07 02 05 ab cd ef -# -# @todo - This should be Vendor-Specific.6809.1.2 = 0xabcdef -# decode-pair - match Vendor-Specific.6809.1 = { 2 = 0xabcdef } encode-pair 26.6809.1.2.3 = 0xabcdef match 1a 0f 00 00 1a 99 01 09 02 07 03 05 ab cd ef -# -# @todo - This should be Vendor-Specific.6809.1 = { 2 = { 3 = 0xabcdef } } -# decode-pair - match Vendor-Specific.6809.1 = { 2 = { 3 = 0xabcdef } } diff --git a/src/tests/unit/xlat/base.txt b/src/tests/unit/xlat/base.txt index 571f9a2826b..63706b75d66 100644 --- a/src/tests/unit/xlat/base.txt +++ b/src/tests/unit/xlat/base.txt @@ -304,9 +304,6 @@ match [0]{ echo }, [1]{ hello }, [2]{ %{Tmp-String-0}:1234 }, [3]{ world } xlat %(debug: 5) match %(debug: 5) -# -# @todo - debug takes an integer, and this is wrong. -# xlat %(debug: "foo") match %(debug: "foo") @@ -319,7 +316,7 @@ match %(rpad:&User-Name 5 x) # # The second argument should be an integer. # -# @todo - we don't currently track string offsets for intermediate nodes, +# @todo - parsing - we don't currently track string offsets for intermediate nodes, # so the "offset 23" is wrong. It also doesn't say *which* string is wrong. We'll fix that later. # xlat %(rpad:&User-Name foo x) diff --git a/src/tests/unit/xlat/cond_base.txt b/src/tests/unit/xlat/cond_base.txt index 41646f9fb8f..08266fa60b7 100644 --- a/src/tests/unit/xlat/cond_base.txt +++ b/src/tests/unit/xlat/cond_base.txt @@ -116,7 +116,7 @@ xlat_purify ((!"foo") == false) match true # -# @todo - add a flag which says to parse the FULL thing, or only parse part of it? +# @todo - unit_test_attribute - add a flag which says to parse the FULL thing, or only parse part of it? # xlat_purify ((&User-Name == &Filter-Id) || (&Reply-Message == &User-Password))) match Passed in 67 characters, but only parsed 66 characters @@ -183,7 +183,7 @@ match ((ipaddr)&Filter-Id == &Framed-IP-Address) # We can automatically promote things as needed. But if the # user forces incompatible types, then that's an error. # -# @todo - perhaps better errors for casts? +# @todo - parsing - better errors for casts # xlat_purify &Filter-Id == &Framed-IP-Address match ERROR offset 23: No operand found. Expected &ref, literal, 'quoted literal', "%{expansion}", or enum value @@ -228,7 +228,7 @@ match (&Session-Timeout == 10) # Automatic type inference means this is fine # -# @todo - in binary resolve, cast the RHS to the type of the LHS. +# @todo - peephole - resolve the RHS to the type of the LHS # xlat_purify &Session-Timeout == '10' match (&Session-Timeout == '10') @@ -248,7 +248,7 @@ xlat_purify X == 1 match ERROR offset 10: Failed parsing string as type 'uint32' # -# @todo - resolution is delayed, so we don't know where in the input +# @todo - parsing - resolution is delayed, so we don't know where in the input # string the RHS is. # xlat_purify &NAS-Port == X @@ -294,7 +294,7 @@ match ERROR offset 12: Missing separator, expected ':' xlat_purify true match true -# @todo - for conditions, this should evaluate to "true". However, this evaluation +# @todo - parsing - for conditions, this should evaluate to "true". However, this evaluation # only occurs in the condition code, and not in the xlat code! xlat_purify 1 match 1 @@ -402,10 +402,6 @@ match 4 xlat_purify ('a') match 'a' -# -# @todo - modules && return codes. -# - #xlat_purify (a) #match ERROR offset 1: Expected a module return code @@ -447,7 +443,7 @@ match ("foo" == &User-Name) # This used to be expr, but expr isn't a builtin, so it failed... # -# @todo - arguably this is a failed thing, we should get: +# @todo - peephole - arguably this is a failed thing, we should get: # # ERROR: Failed casting 0x002ade8665c69219ca16bd108d92c8d5 to data type uint32: Invalid cast from octets to uint32. Source length 16 is greater than destination type size 4 # @@ -526,7 +522,7 @@ match (&Tmp-uint64-0 > &Session-Timeout) # # Parse OIDs into known attributes, where possible. # -# @todo - whoops, resolve it +# @todo - peephole - resolve the unknown attribute to something real! xlat_purify &26.24757.84.9.5.4 == 0x1a99 match (&26.24757.84.9.5.4 == 0x1a99) #match &Vendor-Specific.WiMAX.Packet-Flow-Descriptor-v2.Classifier.Src-Spec.Port == 6809 @@ -589,12 +585,12 @@ match false # # 'Unknown' attributes which are defined in the main dictionary # should be resolved to their real names. -# @todo - resolve! +# @todo - peephole - resolve it to something real xlat_purify &1 == 0x616263 match (&1 == 0x616263) #match (&User-Name == 'abc') -# @todo - resolve! +# @todo - peephole - resolve it to something real #xlat_purify &26.11344.1 == 0x7f000001 #match &Vendor-Specific.FreeRADIUS.Proxied-To == 127.0.0.1 @@ -618,9 +614,8 @@ xlat_purify 192.168.0.0/16 > 192.168.1.2 match true -# @todo - fix up cast! -#xlat_purify 192.168.0.0/16 > 192.168.1.2 -#match true +xlat_purify 192.168.0.0/16 > 192.168.1.2 +match true xlat_purify &NAS-IP-Address == 192.168.0.0/24 match ((ipv4prefix)&NAS-IP-Address == 192.168.0.0/24) @@ -636,7 +631,6 @@ match (192.168.0.0/24 > &NAS-IP-Address) # # This is allowed and means "the list is not empty" # -# @todo - we need a "parse as condition" flag! xlat_purify (&reply) match &reply @@ -650,7 +644,7 @@ match false # # Attributes with a protocol namespace # -# @todo - if the explicit namespace is the same as the implicit one, we can omit +# @todo - normalization - if the explicit namespace is the same as the implicit one, we can omit # the explicit one? But this is largely due to the printing fixes, where we just # print the tmpl name as-is. xlat_purify &radius.User-Name == 'bob' @@ -702,10 +696,6 @@ match 1 xlat_purify 1 || 2 || (&User-Name == "bob") match 1 -# -# @todo - this is arguably incorrect. It could return -# "true", or "1". -# xlat_purify (&User-Name == "bob") || 1 || 2 match 1 @@ -789,4 +779,4 @@ xlat_purify (192.168.0.1 !== 192.168.0.2) match true count -match 330 +match 332 diff --git a/src/tests/unit/xlat/cond_regex.txt b/src/tests/unit/xlat/cond_regex.txt index ba2d6fa5492..376ccc47f27 100644 --- a/src/tests/unit/xlat/cond_regex.txt +++ b/src/tests/unit/xlat/cond_regex.txt @@ -12,7 +12,6 @@ match ERROR offset 16: Expected regular expression xlat_purify (&User-Name == /foo/) match ERROR offset 16: Unexpected regular expression -# @todo - this should be allowed? xlat_purify &User-Name =~ &Filter-Id match ERROR offset 15: Expected regular expression diff --git a/src/tests/unit/xlat/purify.txt b/src/tests/unit/xlat/purify.txt index 852643e11ef..c10b6f3c4ae 100644 --- a/src/tests/unit/xlat/purify.txt +++ b/src/tests/unit/xlat/purify.txt @@ -139,7 +139,7 @@ xlat_purify 1 < (uint32) 2 match true # -# @todo - for exec, xlat, etc., if we're doing an existence check of +# @todo - peephole - for exec, xlat, etc., if we're doing an existence check of # string / octets, then the check is for "length>0", NOT for parsing # the contents of the data type. #