Bug 6162 - Template TAGS don't allow underscores
Summary: Template TAGS don't allow underscores
Status: NEW
Alias: None
Product: Spamassassin
Classification: Unclassified
Component: Libraries (show other bugs)
Version: 3.2.5
Hardware: Other All
: P5 trivial
Target Milestone: 4.0.0
Assignee: SpamAssassin Developer Mailing List
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-07-21 10:12 UTC by Karsten Bräckelmann
Modified: 2019-08-01 20:19 UTC (History)
4 users (show)



Attachment Type Modified Status Actions Submitter/CLA Status
Add word boundaries to tag regex patch None Kevin Golding [NoCLA]

Note You need to log in before you can comment on or make changes to this bug.
Description Karsten Bräckelmann 2009-07-21 10:12:41 UTC
$pms->set_tag ('TAG_WITH_UNDERSCORE', 'value');

does return the actual value, suggesting the tag indeed has been defined, as per M::SA::PerMsgStatus set_tag() documentation, since the returned value is not undef.

Using a a template with underscores however, is not expanded to the value, but used literally.

  add_header all  Foo  _TAG_WITH_UNDERSCORES_

results in the following header in the mail.

  X-Spam-Foo: _TAG_WITH_UNDERSCORES_


$pms->{tag_data}->{TAG_WITH_UNDERSCORES} does hold the correct 'value'. And the winner is...

Somewhere between _process_header(), _replace_tags() and _get_tag(), it seems. After quite a long hunt, I believe I finally got the little bugger.

PerMsgStatus _replace_tags(), who came up with *that* wonderful line of code?

  $text =~ s{(_(\w+?)(?:\((.*?)\))?_)}{

The fact that RE is not anchored, but the $2 match is not greedy, results in the RE missing TAG_WITH_UNDERSCORE. Did not test this, though, because...

Any fix, if any, needs to carefully consider bug 4793.
Comment 1 Kevin Golding 2015-12-31 11:50:09 UTC
Created attachment 5365 [details]
Add word boundaries to tag regex

Adds word boundaries to the start and end of the tag regex to enable underscores within a template tag.

Checks for a double underscore so that multiple tags can be squashed together without a space/punctuation if desired.

The regex along doesn't even match many of the test cases given in 4793 which avoids issues with that, and the matches it does give will resolve to an unrecognised tag and therefore the default of not changing those as introduced in the fixes would come into effect.
Comment 2 Kevin A. McGrail 2015-12-31 13:50:38 UTC
Thanks Kevin.  It's a trivial regex and nice you made all the fixes.  We'll look at this post Holiday.
Comment 3 Henrik Krohns 2019-08-01 11:38:41 UTC
Allowing underscores inside a tag would be _horrible_ design, given that underscore is the separator. Just looking at any set_tag in SA sources clearly show the spirit of naming is intended to be _JUSTCAPS_.

I vote making set_tag to reject anything else than _ALPHANUM(foo)_ for trunk/4.0.0 and clarifying this in documentation.
Comment 4 Kevin A. McGrail 2019-08-01 19:19:33 UTC
+1
Comment 5 Bill Cole 2019-08-01 20:19:10 UTC
(In reply to Henrik Krohns from comment #3)
> Allowing underscores inside a tag would be _horrible_ design, given that
> underscore is the separator. Just looking at any set_tag in SA sources
> clearly show the spirit of naming is intended to be _JUSTCAPS_.
> 
> I vote making set_tag to reject anything else than _ALPHANUM(foo)_ for
> trunk/4.0.0 and clarifying this in documentation.

+!