On Tue, Feb 20, 2024 at 05:10:52PM +0100, Omar Polo wrote:
> Now that the parser was unified, let's fix the original issue.  The
> parser splits the line on the ':' character too for key-value tables,
> which means that ipv6 addresses are misparsed.  The "::1 localhost"
> example given at the bottom of table(5) is actually parsed as key "" and
> value ":1 localhost".
> 
> For list tables, the special comment "# @list" can be used to turn off
> the automatic key/value splitting, but for key-valued tables the parser
> needs to be fixed.
> 
> Furthermore, smtpd serializes ipv6 addresses by wrapping them in braces
> before querying the tables, so even if "::1 localhost" worked, it would
> still not match at runtime as ::1 doesn't match [::1].
> 
> The manpage also lists ipv6:::1 as example, that is similarly broken.
> 
> So, I'm proposing to require ipv6 addresses to be wrapped in braces in
> table(5) files, and to amend the parser to skip over to the matching ]
> if the line starts with [.  Is it enough or should it be more strict?
> 

I dunno if it should be more strict but that would already be a good
start and an improvement over today.


> Before the diff: (only the first two entries are interesting, the rest
> is just there to make sure I'm not breaking existing behaviours)
> 
> 	suzaku% cat t
> 	[::1]: bar
> 	[::2]   bar
> 	foo1    bar
> 	foo2:   bar
> 	foo3:bar
> 	foo4 :  bar
> 	foo5 :: bar
> 	foo6::bar
> 	foo7:: bar
> 	suzaku% makemap t && makemap -U t.db
> 	makemap: t:2: duplicate entry for [
> 
> after:
> 
> 	suzaku% ln -s smtpctl/obj/smtpctl makemap
> 	suzaku% ./makemap t && ./makemap -U t.db | sort
> 	[::1]   bar
> 	[::2]   bar
> 	foo1    bar
> 	foo2    bar
> 	foo3    bar
> 	foo4    bar
> 	foo5    :: bar
> 	foo6    :bar
> 	foo7    bar
> 
> with the exception of the [::1] and [::2] lines, the rest of the table
> is still parsed exactly as before.
> 
> thoughs?
> 

Generally ok with this but also starting to wonder if makemap and static
tables parsing would not benefit from a regex(3)-based line parser to be
honest. It's not used anywhere an expensive regex would be an issue, and
it would be more maintainable than having to read it every ten year when
someone hits a case we didn't hit before.

Dunno, just opening the discussion

-- 
Gilles Chehade