https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6274

           Summary: Semantics of CONF_TYPE_* constants and additional
                    config argument types
           Product: Spamassassin
           Version: 3.3.0
          Platform: All
        OS/Version: All
            Status: NEW
          Severity: enhancement
          Priority: P5
         Component: Libraries
        AssignedTo: dev@spamassassin.apache.org
        ReportedBy: mark.marti...@ijs.si


Carrying over out-of-scope topics from Bug 5895:



Bug 5895 comment 10 (Mark):

The Parser.pm claims:
It is good practice to set a 'type', if possible, describing how your settings
are stored on the Conf object; this allows the SpamAssassin test suite to
validate that the settings do not 'leak' between users.

so I'd say that a:
  type => $CONF_TYPE_STRING,
should be added to the originating_ip_headers and clear_originating_ip_headers
setting, not the test tweaked.

(or better yet, a $CONF_TYPE_NOARG type invented for settings like
clear_headers, clear_trusted_networks, clear_internal_networks,
clear_msa_networks, clear_originating_ip_headers, clear_report_template,
clear_unsafe_report_template)


Bug 5895 comment 11 (Sidney):

I tried adding type => $CONF_TYPE_STRING, to originating_ip_headers but it
doesn't work because originating_ip_headers takes an array of strings, not a
string. Similarly, there is no constant to use for options like
clear_originating_ip_headers that take no argument.

As you suggested, we can create new types and eliminate the corresponding
entries in the list in t/cross_user_config_leak.t, ans I'm +1 for doing that,
but that's outside the scope of this bug.


Bug 5895 comment 18 (Mark):

(In reply to comment #16)
> I didn't look into the cause and it could very well have been my mistake
> when changing things to test it, but this is what I got when I declared
> originating_ip_headers as a string type, removed it from the test exclude
> list, then ran just the one test:
> 
> $ make test TEST_FILES=t/cross_user_config_leak.t
> t/cross_user_config_leak.t .. 1/6 Can't use string ("__test_expected_str")
> as an ARRAY ref while "strict refs" in use
> at ../lib/Mail/SpamAssassin/Conf.pm line 3910.

Thanks. Now that rises a question, is a constant $CONF_TYPE_STRING
(and its sisters) supposed to tell the syntax of arguments in a config file,
or is it supposed to tell the internal data structure used to represent
these arguments???


Bug 5895 comment 20 (Sidney):

Looking over the test code I think the type would have to indicate the internal
representation. In order to read in the config file we have to call the
routines that are set to read it, which contain embedded methods to parse the
text file. This is especially true for this test which is checking for leaks
between loads of the configuration, as it has to actually load the
configuration and check that the internal data structures are not leaking
through.

Given that, the solution will be to define a string array type and a null type
so that we can remove options such as trusted_networks, originating_ip_headers
for the former, and the clear_* options for the latter. Of course the null type
should cause the option to be skipped just like if it is in the ignore list.

That will leave a few more options still in the ignore list, but it will be a
good start.

-- 
Configure bugmail: 
https://issues.apache.org/SpamAssassin/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

Reply via email to