| Message ID | 20200206113828.7306-2-phil@nwl.cc |
|---|---|
| State | Changes Requested |
| Delegated to: | Pablo Neira |
| Headers | show |
| Series | [nft,1/2] doc: nft.8: Mention wildcard interface matching | expand |
On Thu, Feb 06, 2020 at 12:38:28PM +0100, Phil Sutter wrote: > Accept sole escaped asterisks as well as unescaped asterisks if > surrounded by strings. The latter is merely cosmetic, but literal > asterisk will help when translating from iptables where asterisk has no > special meaning. > > Signed-off-by: Phil Sutter <phil@nwl.cc> > --- > src/scanner.l | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/scanner.l b/src/scanner.l > index 99ee83559d2eb..da9bacee23eb5 100644 > --- a/src/scanner.l > +++ b/src/scanner.l > @@ -120,7 +120,7 @@ numberstring ({decstring}|{hexstring}) > letter [a-zA-Z] > string ({letter}|[_.])({letter}|{digit}|[/\-_\.])* > quotedstring \"[^"]*\" > -asteriskstring ({string}\*|{string}\\\*) > +asteriskstring ({string}\*|{string}\\\*|\\\*|{string}\*{string}) Probably this: {string}\\\*{string}) instead of: {string}\*{string}) ? The escaping makes it probably clear that there is no support for infix wildcard matching? This asteriskstring rule is falling under the string rule in bison. This is allowing to use \\\* for log messages too, and elsewhere.
Hi Pablo, On Fri, Feb 07, 2020 at 06:31:40PM +0100, Pablo Neira Ayuso wrote: > On Thu, Feb 06, 2020 at 12:38:28PM +0100, Phil Sutter wrote: > > Accept sole escaped asterisks as well as unescaped asterisks if > > surrounded by strings. The latter is merely cosmetic, but literal > > asterisk will help when translating from iptables where asterisk has no > > special meaning. > > > > Signed-off-by: Phil Sutter <phil@nwl.cc> > > --- > > src/scanner.l | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/src/scanner.l b/src/scanner.l > > index 99ee83559d2eb..da9bacee23eb5 100644 > > --- a/src/scanner.l > > +++ b/src/scanner.l > > @@ -120,7 +120,7 @@ numberstring ({decstring}|{hexstring}) > > letter [a-zA-Z] > > string ({letter}|[_.])({letter}|{digit}|[/\-_\.])* > > quotedstring \"[^"]*\" > > -asteriskstring ({string}\*|{string}\\\*) > > +asteriskstring ({string}\*|{string}\\\*|\\\*|{string}\*{string}) > > Probably this: > > {string}\\\*{string}) > > instead of: > > {string}\*{string}) > > ? > > The escaping makes it probably clear that there is no support for > infix wildcard matching? Ah, you're right. I assumed it wasn't necessary to escape the asterisk mid-string, but if we ever added support for infix wildcards (no matter how unlikely) we were in real trouble. BTW: Given how confusing bison-generated error messages are, maybe I should introduce "infixasteriskstring" in scanner.l to catch unescaped infix asterisks and generate a readable error message from there? > This asteriskstring rule is falling under the string rule in bison. > This is allowing to use \\\* for log messages too, and elsewhere. Ah, that's right. Good, bad, ugly? Just a "neutral remark" from you? :) Thanks, Phil
On Fri, Feb 07, 2020 at 06:59:02PM +0100, Phil Sutter wrote: > Hi Pablo, > > On Fri, Feb 07, 2020 at 06:31:40PM +0100, Pablo Neira Ayuso wrote: > > On Thu, Feb 06, 2020 at 12:38:28PM +0100, Phil Sutter wrote: > > > Accept sole escaped asterisks as well as unescaped asterisks if > > > surrounded by strings. The latter is merely cosmetic, but literal > > > asterisk will help when translating from iptables where asterisk has no > > > special meaning. > > > > > > Signed-off-by: Phil Sutter <phil@nwl.cc> > > > --- > > > src/scanner.l | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/src/scanner.l b/src/scanner.l > > > index 99ee83559d2eb..da9bacee23eb5 100644 > > > --- a/src/scanner.l > > > +++ b/src/scanner.l > > > @@ -120,7 +120,7 @@ numberstring ({decstring}|{hexstring}) > > > letter [a-zA-Z] > > > string ({letter}|[_.])({letter}|{digit}|[/\-_\.])* > > > quotedstring \"[^"]*\" > > > -asteriskstring ({string}\*|{string}\\\*) > > > +asteriskstring ({string}\*|{string}\\\*|\\\*|{string}\*{string}) > > > > Probably this: > > > > {string}\\\*{string}) > > > > instead of: > > > > {string}\*{string}) > > > > ? > > > > The escaping makes it probably clear that there is no support for > > infix wildcard matching? > > Ah, you're right. I assumed it wasn't necessary to escape the asterisk > mid-string, but if we ever added support for infix wildcards (no matter > how unlikely) we were in real trouble. Yes, I don't expect mid-string matching in the future, but you never know, so better reserve this just in case :-) > BTW: Given how confusing bison-generated error messages are, maybe I > should introduce "infixasteriskstring" in scanner.l to catch unescaped > infix asterisks and generate a readable error message from there? bison syntax error reporting is not great, yes. If you think that makes it easier for error reporting as a short term way to address the issue, that's fine with me. > > This asteriskstring rule is falling under the string rule in bison. > > This is allowing to use \\\* for log messages too, and elsewhere. > > Ah, that's right. Good, bad, ugly? Just a "neutral remark" from you? :) Just a remark, no issue.
Hi Pablo, On Sun, Feb 09, 2020 at 11:21:43PM +0100, Pablo Neira Ayuso wrote: [...] > Yes, I don't expect mid-string matching in the future, but you never > know, so better reserve this just in case :-) DONE, please see v2 I just sent. > > BTW: Given how confusing bison-generated error messages are, maybe I > > should introduce "infixasteriskstring" in scanner.l to catch unescaped > > infix asterisks and generate a readable error message from there? > > bison syntax error reporting is not great, yes. If you think that > makes it easier for error reporting as a short term way to address the > issue, that's fine with me. Tried, but didn't go well - proper error reporting is best put into parser_bison, but there one can't complain about mid-string asterisk "anywhere" but only in defined places. So in others the then known token will make error messages even more confusing. Cheers, Phil
diff --git a/src/scanner.l b/src/scanner.l index 99ee83559d2eb..da9bacee23eb5 100644 --- a/src/scanner.l +++ b/src/scanner.l @@ -120,7 +120,7 @@ numberstring ({decstring}|{hexstring}) letter [a-zA-Z] string ({letter}|[_.])({letter}|{digit}|[/\-_\.])* quotedstring \"[^"]*\" -asteriskstring ({string}\*|{string}\\\*) +asteriskstring ({string}\*|{string}\\\*|\\\*|{string}\*{string}) comment #.*$ slash \/
Accept sole escaped asterisks as well as unescaped asterisks if surrounded by strings. The latter is merely cosmetic, but literal asterisk will help when translating from iptables where asterisk has no special meaning. Signed-off-by: Phil Sutter <phil@nwl.cc> --- src/scanner.l | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)