You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@spamassassin.apache.org by bu...@bugzilla.spamassassin.org on 2010/03/03 17:11:16 UTC

[Bug 6362] New: Change (urirhssub) mask syntax to netmask

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

           Summary: Change (urirhssub) mask syntax to netmask
           Product: Spamassassin
           Version: SVN Trunk (Latest Devel Version)
          Platform: All
        OS/Version: All
            Status: NEW
          Severity: enhancement
          Priority: P5
         Component: Plugins
        AssignedTo: dev@spamassassin.apache.org
        ReportedBy: jeffc@surbl.org


(broken out from 6335)

--- Comment #41 from Jeff Chan <je...@surbl.org> 2010-03-03 15:41:22 UTC ---
(In reply to comment #31)
> > uridnssub URIBL_DBL dbl.spamhaus.org. A 2130706688
> > 
> > what is 2130706688 ?
> 
> Decimal for 0x7f000100, i.e. 127.0.1.0 mask.
> 
> man Mail::SpamAssassin::Plugin::URIDNSBL
> 
> The mask is poorly documented in that POD, the text does not
> explain what does it actually do, and whether ones or zeroes
> have the desired meaning (the classical Cisco dilema).

I presume a netmask for the above would be 128.255.254.255 which looks much
more reasonable than decimal or hex notation.

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

[Bug 6362] [review] Change (urirhssub) mask syntax to netmask

Posted by bu...@issues.apache.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6362

--- Comment #41 from Mark Martinec <Ma...@ijs.si> 2010-06-03 13:26:51 EDT ---
> > This has been released with 3.3.1, along the Spamhaus DBL support.
> > The test-case has been kept trunk only, because it requires adding a new config
> > option. See the comments I referenced earlier.
> 
> Ahh. Then I rescind my statement that a test case should be a blocker. However,
> it would be nice to get a test into 3.3.2.  

The change to DnsResolver.pm is nontrivial and adds new functionality - which
seems
plenty for a minor patch release like 3.3.2. Considering that the main need for
it is adding a test, and that there are three other DNS-related changes
targeted
for 3.4 (comment 31 and c32), I'd feel more comfortable if they'd go all
together
into 3.4. I don't consider absence of a test in 3.3 is warranting the
nontrivial
change in code. Also, the test is already in trunk and is being evaluated by
Hudson I believe.

> Is this something that would be a net_test and do you expect user configuration
> or a spamassassin project dictate of the config options?

Not sure I understand the question. The test spawns a self-contained DNS
resolver
with wired-in test zone. The missing (hereby added) functionality is to make
SA use that DNS server (for the test) instead of whatever is in
/etc/resolv.conf.
Don't know if that qualifies for a net test or not: it requires a working
TCP/IP stack on the loopback interface, but does not require network
connectivity.

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

[Bug 6362] Change (urirhssub) mask syntax to netmask

Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6362

Mark Martinec <Ma...@ijs.si> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P5                          |P3
   Target Milestone|Undefined                   |3.3.1

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

[Bug 6362] Change (urirhssub) mask syntax to netmask

Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6362

--- Comment #7 from Mark Martinec <Ma...@ijs.si> 2010-03-04 13:35:12 UTC ---
concentrating this topic here:

Bug 6335 comment 69, Justin Mason 2010-03-04 12:14:27 UTC :

Regarding the IP mask -- 0x7f000100 was chosen since initial plans were to
return 127.0.0.2 for the "don't do that" listings for IP addresses.  That would
not match those, instead matching solely the 127.0.1.2 "real" listings.  Since
then, though, Spamhaus have instead gone with using 127.0.1.255, which is in
the 127.0.1.x space, rendering that attempt futile.

We could therefore now just use 2 as the mask for the URIBL_DBL rule, with the
same effect and causing less confusion.  devs, thoughts?

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

[Bug 6362] Change (urirhssub) mask syntax to netmask

Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6362

--- Comment #4 from Mark Martinec <Ma...@ijs.si> 2010-03-03 23:20:35 UTC ---
> > Decimal for 0x7f000100, i.e. 127.0.1.0 mask.
> I presume a netmask for the above would be 128.255.254.255 which looks much
> more reasonable than decimal or hex notation.

Back to the original question: why was the mask chosen as 127.0.1.0,
i.e. why clear the first bit, why clear the second byte (from left),
and why clear all but the last bit in the third byte? Seems to me
the whole mask logic was not well thought out. Perhaps we'd need two
parameters: a mask, and an IPv4 address to match the result against,
as is usual with ACL and similar network settings.

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

[Bug 6362] [review] Change (urirhssub) mask syntax to netmask

Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6362

--- Comment #19 from Mark Martinec <Ma...@ijs.si> 2010-03-09 20:36:17 UTC ---
> Hudson won't be happy, working on it...

r921078, r921096
I believe it will now be alright, passes tests here, still waiting for Hudson.

I find it strange that SA::DnsResolver::connect_sock() is called from
SA::init() -> SA::DnsResolver::reinit_post_fork() without going through
reshuffling and testing DNS servers first by SA::Dns::is_dns_available().
Anyway, the new code is now bug-for-bug compatible with old in this respect.

Also, removed a call to connect_sock_if_reqd() in SA::DnsResolver::nameservers,
I don't think it has any purpose there, overloads the routine's functionality,
and potentially makes a recursive call to connect_sock().

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

[Bug 6362] [review] Change (urirhssub) mask syntax to netmask

Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6362

--- Comment #18 from Mark Martinec <Ma...@ijs.si> 2010-03-09 19:11:32 UTC ---
Hudson won't be happy, working on it...

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

[Bug 6362] [review] Change (urirhssub) mask syntax to netmask

Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6362

Karsten Bräckelmann <gu...@rudersport.de> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
              Group|security                    |
          Component|Security                    |Libraries
         AssignedTo|security@spamassassin.apach |dev@spamassassin.apache.org
                   |e.org                       |

--- Comment #30 from Karsten Bräckelmann <gu...@rudersport.de> 2010-03-23 17:42:34 UTC ---
Moving back off of Security, which got changed by accident during the mass
Target Milestone move.

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

[Bug 6362] [review] Change (urirhssub) mask syntax to netmask

Posted by bu...@issues.apache.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6362

Sidney Markowitz <si...@sidney.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|3.4.0                       |3.3.2

--- Comment #36 from Sidney Markowitz <si...@sidney.com> 2010-05-25 19:19:02 EDT ---
Targeting to 3.3.2. Looks like target was changed to 3.4.0 instead of 3.3.2 by
mistake, this is already committed to trunk (future 3.4 branch)

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

[Bug 6362] Change (urirhssub) mask syntax to netmask

Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6362

--- Comment #9 from Mark Martinec <Ma...@ijs.si> 2010-03-05 00:20:11 UTC ---
The change is fully compatible with existing uses - rolling it
into the trunk:

Bug 6362: Change (urirhssub) mask syntax to netmask;
The patch extends the syntax of a sub-test parameter
in URIDNSBL tests, which now understands ranges and
IP with mask pairs.
Sending lib/Mail/SpamAssassin/Plugin/URIDNSBL.pm
Committed revision 919245.

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

[Bug 6362] [review] Change (urirhssub) mask syntax to netmask

Posted by bu...@issues.apache.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6362

Kevin A. McGrail <km...@pccc.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |kmcgrail@pccc.com

--- Comment #38 from Kevin A. McGrail <km...@pccc.com> 2010-05-26 09:08:43 EDT ---
(In reply to comment #37)
> Actually, the functionality has been committed to the 3.3 branch (comment 23).
> The target has been moved to 3.4 merely for adding a test-case, which requires
> additional functionality (comment 31).
> 
> Moving back to target 3.4. Or did I mis-understand something?

I think a test case should be a blocker for release and move this back to
3.3.2.

KAM

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

[Bug 6362] [review] Change (urirhssub) mask syntax to netmask

Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6362

Justin Mason <jm...@jmason.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P1
             Blocks|                            |6335

--- Comment #14 from Justin Mason <jm...@jmason.org> 2010-03-08 12:09:27 UTC ---
this is now a requirement of bug 6335.

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

[Bug 6362] Change (urirhssub) mask syntax to netmask

Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6362

--- Comment #11 from Justin Mason <jm...@jmason.org> 2010-03-05 16:35:03 UTC ---
ok, +1

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

[Bug 6362] [review] Change (urirhssub) mask syntax to netmask

Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6362

Karsten Bräckelmann <gu...@rudersport.de> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|Libraries                   |Plugins

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

[Bug 6362] Change (urirhssub) mask syntax to netmask

Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6362

--- Comment #10 from Mark Martinec <Ma...@ijs.si> 2010-03-05 00:36:19 UTC ---
Btw, there is a new can() function: has_subtest_for_ranges()
in plugin URIDNSBL if someone is keen on augmenting the DBL rules.

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

[Bug 6362] [review] Change (urirhssub) mask syntax to netmask

Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6362

--- Comment #28 from Justin Mason <jm...@jmason.org> 2010-03-15 10:51:57 UTC ---
(In reply to comment #27)
> Suggesting to leave out this test from 3.3.1 and apply it later
> when the issues pointed out are better understood. I think it suffices
> for the time being to have it in the trunk and that it passes there.

+1.


(fwiw, I would prefer random port assignment if possible, but it can wait ;)

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

[Bug 6362] Change (urirhssub) mask syntax to netmask

Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6362

--- Comment #2 from Mark Martinec <Ma...@ijs.si> 2010-03-03 17:40:32 UTC ---
Created an attachment (id=4694)
 --> (https://issues.apache.org/SpamAssassin/attachment.cgi?id=4694)
Allow mask be specified in hex for URIDNSBL rules

Here is a quick fix - at least allowing a mask to be in hex - before
some more general solution is provided.

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

[Bug 6362] Change (urirhssub) mask syntax to netmask

Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6362

--- Comment #1 from Mark Martinec <Ma...@ijs.si> 2010-03-03 16:29:07 UTC ---
> I presume a netmask for the above would be 128.255.254.255 which looks much
> more reasonable than decimal or hex notation.

As I understand it, there needs to be some syntactic distinction between
a mask and an exact match IP address in the last parameter of the uridnssub.
Currently the distinction is quad-dot vs. integer. I agree this looks
ugly - as a minimum the mask should be specifiable in hex.

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

[Bug 6362] [review] add conf directives dns_server and clear_dns_servers (and a test for: Change (urirhssub) mask syntax to netmask)

Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6362

Darxus <Da...@ChaosReigns.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |Darxus@ChaosReigns.com

--- Comment #43 from Darxus <Da...@ChaosReigns.com> 2011-05-23 19:35:52 UTC ---
It looks like this was committed to trunk and 3.3, and was only being held up
by a test case for trunk, and "...the test is already in trunk and is being
evaluated by Hudson I believe." - comment 41.
So this should be closed.

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

[Bug 6362] [review] Change (urirhssub) mask syntax to netmask

Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6362

--- Comment #32 from Mark Martinec <Ma...@ijs.si> 2010-03-30 18:00:37 UTC ---
> Moving adding of a config directive "dns_server" along with the accompanying
> test case to 3.4.0.  There are some other new DNS features, so it makes
> sense to roll them out together:
> - Bug 6338 brings: dns_options dns0x20, and makes dns_options negateable;
> - Bug 6221 brings: dns_local_ports_permit, dns_local_ports_avoid, and
>     dns_local_ports_none

Should also mention the:
  - Bug 6385 - Instant failover from a dead DNS resolver
to go with the flock.

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

[Bug 6362] [review] Change (urirhssub) mask syntax to netmask

Posted by bu...@issues.apache.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6362

--- Comment #39 from Karsten Bräckelmann <gu...@rudersport.de> 2010-05-26 10:37:21 EDT ---
This has been released with 3.3.1, along the Spamhaus DBL support.

The test-case has been kept trunk only, because it requires adding a new config
option. See the comments I referenced earlier.

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

[Bug 6362] [review] Change (urirhssub) mask syntax to netmask

Posted by bu...@issues.apache.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6362

--- Comment #40 from Kevin A. McGrail <km...@pccc.com> 2010-05-26 13:01:14 EDT ---
(In reply to comment #39)
> This has been released with 3.3.1, along the Spamhaus DBL support.
> 
> The test-case has been kept trunk only, because it requires adding a new config
> option. See the comments I referenced earlier.

Ahh. Then I rescind my statement that a test case should be a blocker. However,
it would be nice to get a test into 3.3.2.  

Is this something that would be a net_test and do you expect user configuration
or a spamassassin project dictate of the config options?

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

[Bug 6362] [review] Change (urirhssub) mask syntax to netmask

Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6362

Mark Martinec <Ma...@ijs.si> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|Change (urirhssub) mask     |[review] Change (urirhssub)
                   |syntax to netmask           |mask syntax to netmask
  Status Whiteboard|                            |needs 1 vote

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

[Bug 6362] [review] Change (urirhssub) mask syntax to netmask

Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6362

--- Comment #26 from Mark Martinec <Ma...@ijs.si> 2010-03-14 00:15:52 UTC ---
> Couldn't create UDP socket: Address already in use at
> main::dns_server(127.0.0.1, 5353)

Are you running something (like a multicasting DNS server) on a
loopback interface at UDP port 5353 ???

Any free port would do, I just picked a 5353 as a less likely candidate
for being in use. I don't think the Net::DNS::Nameserver is capable of
choosing a random free port on its own. We could either:
- choose some other unlikely port, perhaps 53535;
- allow a port number to be specified by an environment variable;
- or add code to try choosing some free port before starting a test
  (and hoping to a void a race condition between a check and listen).

> called at t/dnsbl_subtests.t line 274
> ok 1
> plugin: failed to parse plugin (from @INC): Can't locate
> Mail/SpamAssassin/Plugin/FreeMail.pm in @INC (@INC contains: lib t . /etc/perl
> /usr/local/lib/perl/5.8.8 /usr/local/share/perl/5.8.8 /usr/lib/perl5
> /usr/share/perl5 /usr/lib/perl/5.8 /usr/share/perl/5.8 /usr/local/lib/site_perl
> /usr/local/lib/perl/5.8.7 /usr/local/share/perl/5.8.7) at (eval 88) line 1.
> 
> Can't locate object method "timer_reset" via package "Mail::SpamAssassin" at
> t/dnsbl_subtests.t line 226.
> dubious

I'd need some suggestions here. Seems as if the the "use lib '.'; use lib 't'"
does not accomplish its job: if SpamAssassin 3.2.* is installed, the test
picks old installed SA modules instead of the ones in a package being tested,
which is why it fails on FreeMail.pm and timer_reset.

If you install 3.3.* and then repeat the test, it should pass.

Don't know where the problem lies. Please advise.

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

[Bug 6362] [review] add conf directives dns_server and clear_dns_servers (and a test for: Change (urirhssub) mask syntax to netmask)

Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6362

Mark Martinec <Ma...@ijs.si> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED

--- Comment #44 from Mark Martinec <Ma...@ijs.si> 2011-05-23 20:57:45 UTC ---
> It looks like this was committed to trunk and 3.3, and was only being held up
> by a test case for trunk, and "...the test is already in trunk and is being
> evaluated by Hudson I believe." - comment 41.
> So this should be closed.

Indeed. Closing.

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

[Bug 6362] [review] Change (urirhssub) mask syntax to netmask

Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6362

Justin Mason <jm...@jmason.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P1                          |P2

--- Comment #24 from Justin Mason <jm...@jmason.org> 2010-03-11 17:58:59 UTC ---
I think we can now move this off P1 (required for 3.3.1 release), right?
feel free to put back to P1 if incorrect.

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

[Bug 6362] Change (urirhssub) mask syntax to netmask

Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6362

Jeff Chan <je...@surbl.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jeffc@surbl.org
           See Also|                            |https://issues.apache.org/S
                   |                            |pamAssassin/show_bug.cgi?id
                   |                            |=6335

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

[Bug 6362] [review] Change (urirhssub) mask syntax to netmask

Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6362

Mark Martinec <Ma...@ijs.si> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|3.3.2                       |3.4.0

--- Comment #31 from Mark Martinec <Ma...@ijs.si> 2010-03-30 17:57:58 UTC ---
(In reply to comment #23)
> As for the second patch (a test, along with adding a config option
> "dns_server", needed by this test), I'd like to receive another
> approval or comment.

Moving adding of a config directive "dns_server" along with the accompanying
test case to 3.4.0.  There are some other new DNS features, so it makes
sense to roll them out together:

- Bug 6338 brings: dns_options dns0x20, and makes dns_options negateable;
- Bug 6221 brings: dns_local_ports_permit, dns_local_ports_avoid, and
    dns_local_ports_none

Btw, all the new options mentioned here are already in the trunk.

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

[Bug 6362] [review] Change (urirhssub) mask syntax to netmask

Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6362

--- Comment #13 from Justin Mason <jm...@jmason.org> 2010-03-08 11:55:26 UTC ---
btw, we need tests for this new code added to trunk (at least) before this bug
can be closed.

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

[Bug 6362] [review] Change (urirhssub) mask syntax to netmask

Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6362

--- Comment #23 from Mark Martinec <Ma...@ijs.si> 2010-03-11 11:12:59 UTC ---
> +1

Thanks.

As a first step, I'm only rolling-in the "extends the subtests syntax"
patch (along with a last-minute small fix in Comment 20). This should
give the required functionality to 3.3 required by Bug 6335.

As for the second patch (a test, along with adding a config option
"dns_server", needed by this test), I'd like to receive another
approval or comment.


3.3:
Bug 6362: extend the syntax of a sub-test parameter in URIDNSBL tests
Sending lib/Mail/SpamAssassin/Plugin/URIDNSBL.pm
Committed revision 921800.

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

[Bug 6362] [review] add conf directives dns_server and clear_dns_servers (and a test for: Change (urirhssub) mask syntax to netmask)

Posted by bu...@issues.apache.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6362

Mark Martinec <Ma...@ijs.si> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[review] Change (urirhssub) |[review] add conf
                   |mask syntax to netmask      |directives dns_server and
                   |                            |clear_dns_servers (and a
                   |                            |test for: Change
                   |                            |(urirhssub) mask syntax to
                   |                            |netmask)

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

[Bug 6362] [review] Change (urirhssub) mask syntax to netmask

Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6362

Mark Martinec <Ma...@ijs.si> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #4702|0                           |1
        is obsolete|                            |

--- Comment #21 from Mark Martinec <Ma...@ijs.si> 2010-03-10 16:07:13 UTC ---
Created an attachment (id=4707)
 --> (https://issues.apache.org/SpamAssassin/attachment.cgi?id=4707)
merged patch implementing a test, for easier reviewing

The attached patch is a merge of all four yesterday's commits for
easier reviewing. It implements a test dnsbl_subtests.t with its
supporting code in the SA.

So, in summary, there are two patches here:
- the patch attached to Comment 8 extends the subtests syntax;
- the patch attached here only provides a test for the above, along
  with a supporting change in SA (allow to specify a DNS server
  and its port number in a config file)

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

[Bug 6362] [review] Change (urirhssub) mask syntax to netmask

Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6362

Justin Mason <jm...@jmason.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Status Whiteboard|ready for commit            |needs 1 more vote for test

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

[Bug 6362] Change (urirhssub) mask syntax to netmask

Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6362

Justin Mason <jm...@jmason.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jm@jmason.org

--- Comment #6 from Justin Mason <jm...@jmason.org> 2010-03-04 00:18:47 UTC ---
(In reply to comment #4)
> > > Decimal for 0x7f000100, i.e. 127.0.1.0 mask.
> > I presume a netmask for the above would be 128.255.254.255 which looks much
> > more reasonable than decimal or hex notation.
> 
> Back to the original question: why was the mask chosen as 127.0.1.0,
> i.e. why clear the first bit, why clear the second byte (from left),
> and why clear all but the last bit in the third byte? Seems to me
> the whole mask logic was not well thought out. Perhaps we'd need two
> parameters: a mask, and an IPv4 address to match the result against,
> as is usual with ACL and similar network settings.

the initial plan was for "real" DBL listings to use 127.0.1.2, and the "don't
do that" listings for all IP address lookups to use 127.0.0.2.  Given that,
masking with 127.0.1.0 would allow us to not match on the "don't do that"
listings, while matching the "real" listings.

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

[Bug 6362] [review] Change (urirhssub) mask syntax to netmask

Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6362

--- Comment #17 from Mark Martinec <Ma...@ijs.si> 2010-03-09 18:53:56 UTC ---
Created an attachment (id=4702)
 --> (https://issues.apache.org/SpamAssassin/attachment.cgi?id=4702)
patch corresponding to r921052

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

[Bug 6362] [review] Change (urirhssub) mask syntax to netmask

Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6362

--- Comment #35 from Mark Martinec <Ma...@ijs.si> 2010-04-05 23:55:22 UTC ---
trunk:
  Bug 6362: let t/dnsbl_subtests.t choose a random free port
  instead of using a fixed preconfigured port number for the
  spawned test DNS server
Sending t/dnsbl_subtests.t
Committed revision 930986.

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

[Bug 6362] Change (urirhssub) mask syntax to netmask

Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6362

--- Comment #3 from Mark Martinec <Ma...@ijs.si> 2010-03-03 18:08:47 UTC ---
> Here is a quick fix - at least allowing a mask to be in hex - before
> some more general solution is provided.

Applied to trunk, except for the rule change:

Bug 6362: Allow a mask be specified in hex for URIDNSBL rules
Sending lib/Mail/SpamAssassin/Plugin/URIDNSBL.pm
Committed revision 918605.

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

[Bug 6362] [review] Change (urirhssub) mask syntax to netmask

Posted by bu...@issues.apache.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6362

Kevin A. McGrail <km...@pccc.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Status Whiteboard|needs 1 more vote for test  |ready for test

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

[Bug 6362] [review] Change (urirhssub) mask syntax to netmask

Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6362

Warren Togami <wa...@togami.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Status Whiteboard|needs 1 vote                |ready for commit

--- Comment #22 from Warren Togami <wa...@togami.com> 2010-03-11 03:26:12 UTC ---
+1

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

[Bug 6362] [review] Change (urirhssub) mask syntax to netmask

Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6362

--- Comment #12 from Mark Martinec <Ma...@ijs.si> 2010-03-05 17:00:12 UTC ---
Btw, before going to sleep last night I started worrying what happens
with values (or IPv4 addresses) beyond 128.0.0.0, i.e. signed integer
overflow. I tested it today, seems that both the unpack with "N",
and the hex() do the right thing even on 32-bit integer perls, and
return unsigned 32-bit integers, so this works correctly on all hosts
that I tried.

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

[Bug 6362] [review] Change (urirhssub) mask syntax to netmask

Posted by bu...@issues.apache.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6362

Karsten Bräckelmann <gu...@rudersport.de> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|3.3.2                       |3.4.0

--- Comment #37 from Karsten Bräckelmann <gu...@rudersport.de> 2010-05-26 08:45:05 EDT ---
Actually, the functionality has been committed to the 3.3 branch (comment 23).
The target has been moved to 3.4 merely for adding a test-case, which requires
additional functionality (comment 31).

Moving back to target 3.4. Or did I mis-understand something?

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

[Bug 6362] [review] Change (urirhssub) mask syntax to netmask

Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6362

Sidney Markowitz <si...@sidney.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |sidney@sidney.com

--- Comment #33 from Sidney Markowitz <si...@sidney.com> 2010-04-04 07:35:06 UTC ---
Jumping in late because my test setup on this machine has been out of
commission for a while... I'm also getting the UDP port 5353 in use failure on
the test. This is on a MacBook running Mac OS 10.6.3.

$ sudo lsof -i:5353
COMMAND   PID    USER         FD TYPE  DEVICE     SIZE/OFF NODE NAME
mDNSRespo  18 _mdnsresponder  8u IPv4 0x04edce14      0t0  UDP *:mdns
mDNSRespo  18 _mdnsresponder  9u IPv6 0x04edcd38      0t0  UDP *:mdns

Googling for mdnsresponder finds

http://developer.apple.com/networking/bonjour/faq.html

"mDNSResponder is a Bonjour system service that implements Multicast DNS
Service Discovery for discovery of services on the local network"

>From reading that it looks like 5353 is not a good port to use for the test, as
it is likely to become more commonly used, cross-platform.

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

[Bug 6362] Change (urirhssub) mask syntax to netmask

Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6362

--- Comment #5 from Karsten Bräckelmann <gu...@rudersport.de> 2010-03-03 23:52:09 UTC ---
Agreed, exactly what I've thought and mentioned in private discussions before.
As-is, the ANDed bitmask is quite useless. There's no way to enforce valid
results to be in a specific range, for example.

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

[Bug 6362] [review] Change (urirhssub) mask syntax to netmask

Posted by bu...@issues.apache.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6362

--- Comment #42 from Kevin A. McGrail <km...@pccc.com> 2010-06-03 17:02:50 EDT ---
> The change to DnsResolver.pm is nontrivial and adds new functionality - which
> seems
> plenty for a minor patch release like 3.3.2. Considering that the main need for
> it is adding a test, and that there are three other DNS-related changes
> targeted
> for 3.4 (comment 31 and c32), I'd feel more comfortable if they'd go all
> together
> into 3.4. I don't consider absence of a test in 3.3 is warranting the
> nontrivial
> change in code. Also, the test is already in trunk and is being evaluated by
> Hudson I believe.

That makes sense.  I had some other discussions about this and it was my
confusion that this was only for a test case not knowing that this ALSO added
new functionality.

I'm +1 for the code for the 3.4 as mentioned in comments 31 and 32.


> Not sure I understand the question. The test spawns a self-contained DNS
> resolver
> with wired-in test zone. The missing (hereby added) functionality is to make
> SA use that DNS server (for the test) instead of whatever is in
> /etc/resolv.conf.
> Don't know if that qualifies for a net test or not: it requires a working
> TCP/IP stack on the loopback interface, but does not require network
> connectivity.

My question dealt with why we need configuration options for a test case.  Then
I realized it deals with the test and adds new functionality hence the new
options.  You can safely ignore.

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

[Bug 6362] Change (urirhssub) mask syntax to netmask

Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6362

Mark Martinec <Ma...@ijs.si> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #4694|0                           |1
        is obsolete|                            |

--- Comment #8 from Mark Martinec <Ma...@ijs.si> 2010-03-04 21:08:18 UTC ---
Created an attachment (id=4697)
 --> (https://issues.apache.org/SpamAssassin/attachment.cgi?id=4697)
replacement patch

The patch extends the syntax of a sub-test parameter in URIDNSBL tests.

It makes rules like the following possible:

urirhssub URIBL_DBL_SPAM        dbl.spamhaus.org.  A  127.0.1.2
urirhssub URIBL_DBL_SPAM2       dbl.spamhaus.org.  A  127.0.1.3-127.0.1.19
urirhssub URIBL_DBL_PHISH       dbl.spamhaus.org.  A  127.0.1.20-127.0.1.39
urirhssub URIBL_DBL_MALWARE     dbl.spamhaus.org.  A  127.0.1.40-127.0.1.59
urirhssub URIBL_DBL_PROHIBITED  dbl.spamhaus.org.  A  127.0.1.255

>From the URIDNSBL POD:

C<subtest> is a sub-test to run against the returned data.  The sub-test may
be in one of the following forms: m, n1-n2, or n/m, where n,n1,n2,m can be
any of: decimal digits, 0x followed by up to 8 hexadecimal digits, or an IPv4
address in quad-dot form. The 'A' records (IPv4 dotted address) as returned
by DNSBLs lookups are converted into a numerical form (r) and checked against
the specified sub-test as follows:
for a range n1-n2 the following must be true: (r >= n1 && r <= n2);
for a n/m form the following must be true: (r & m) == (n & m);
for a single value in quad-dot form the following must be true: r == n;
for a single decimal or hex form the following must be true: (r & n) != 0.

Some typical examples of a sub-test are: 127.0.1.2, 127.0.1.20-127.0.1.39,
0.0.0.16/0.0.0.16, 0x10/0x10, 16, 0x10 .

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

[Bug 6362] [review] Change (urirhssub) mask syntax to netmask

Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6362

--- Comment #20 from Mark Martinec <Ma...@ijs.si> 2010-03-10 02:03:52 UTC ---
trunk:

Bug 6362 improvements (case-insensitive hex fix, added tests for numeric IPs)
Sending lib/Mail/SpamAssassin/Plugin/URIDNSBL.pm
Sending t/dnsbl_subtests.t
Committed revision 921207.

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

[Bug 6362] [review] Change (urirhssub) mask syntax to netmask

Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6362

--- Comment #15 from Mark Martinec <Ma...@ijs.si> 2010-03-09 02:20:40 UTC ---
> btw, we need tests for this new code added to trunk (at least) before this bug
> can be closed.

Working on it...

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

[Bug 6362] [review] Change (urirhssub) mask syntax to netmask

Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6362

--- Comment #27 from Mark Martinec <Ma...@ijs.si> 2010-03-14 00:52:58 UTC ---
Suggesting to leave out this test from 3.3.1 and apply it later
when the issues pointed out are better understood. I think it suffices
for the time being to have it in the trunk and that it passes there.
(or move it to an xt for 3.3.1)

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

[Bug 6362] [review] Change (urirhssub) mask syntax to netmask

Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6362

--- Comment #34 from Mark Martinec <Ma...@ijs.si> 2010-04-04 09:54:27 UTC ---
> "mDNSResponder is a Bonjour system service that implements Multicast DNS
> Service Discovery for discovery of services on the local network"

Yes, I know. I'll think of a way to let a test server find some free port.
For the time being, just edit the test, replacing the 5353 with something else.

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

[Bug 6362] [review] Change (urirhssub) mask syntax to netmask

Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6362

--- Comment #25 from Justin Mason <jm...@jmason.org> 2010-03-12 14:27:57 UTC ---
(In reply to comment #21)
> Created an attachment (id=4707)
 --> (https://issues.apache.org/SpamAssassin/attachment.cgi?id=4707) [details]
> merged patch implementing a test, for easier reviewing

> - the patch attached here only provides a test for the above, along
>   with a supporting change in SA (allow to specify a DNS server
>   and its port number in a config file)

-1, this isn't passing for me on trunk:

: 176...; prove -v t/dnsbl_subtests.t 
t/dnsbl_subtests....1..47
# Running under perl version 5.008008 for linux
# Current time local: Fri Mar 12 14:26:30 2010
# Current time GMT:   Fri Mar 12 14:26:30 2010
# Using Test.pm version 1.25
Couldn't create UDP socket: Address already in use at
/usr/lib/perl5/Net/DNS/Nameserver.pm line 112
        Net::DNS::Nameserver::new('Net::DNS::Nameserver', 'LocalAddr',
127.0.0.1, 'LocalPort', 5353, 'ReplyHandler', 'CODE(0x83077f8)', 'Verbose', 0,
...) called at t/dnsbl_subtests.t line 211
        main::dns_server(127.0.0.1, 5353) called at t/dnsbl_subtests.t line 274
ok 1
plugin: failed to parse plugin (from @INC): Can't locate
Mail/SpamAssassin/Plugin/FreeMail.pm in @INC (@INC contains: lib t . /etc/perl
/usr/local/lib/perl/5.8.8 /usr/local/share/perl/5.8.8 /usr/lib/perl5
/usr/share/perl5 /usr/lib/perl/5.8 /usr/share/perl/5.8 /usr/local/lib/site_perl
/usr/local/lib/perl/5.8.7 /usr/local/share/perl/5.8.7) at (eval 88) line 1.

Can't locate object method "timer_reset" via package "Mail::SpamAssassin" at
t/dnsbl_subtests.t line 226.
dubious
        Test returned status 29 (wstat 7424, 0x1d00)
DIED. FAILED tests 2-47
        Failed 46/47 tests, 2.13% okay
Failed Test        Stat Wstat Total Fail  Failed  List of Failed
-------------------------------------------------------------------------------
t/dnsbl_subtests.t   29  7424    47   92 195.74%  2-47
Failed 1/1 test scripts, 0.00% okay. 46/47 subtests failed, 2.13% okay.

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

[Bug 6362] [review] Change (urirhssub) mask syntax to netmask

Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6362

--- Comment #16 from Mark Martinec <Ma...@ijs.si> 2010-03-09 18:52:26 UTC ---
> btw, we need tests for this new code added to trunk (at least) before this bug
> can be closed.

It was tricker than anticipated, but here it is. Please review carefully.

trunk:

Bug 6362: Change urirhssub mask syntax:
- provided a supporting test t/dnsbl_subtests.t;
- provided config directives dns_server and clear_dns_servers,
  offering means to specify a list of DNS servers with
  optional port numbers, needed by dnsbl_subtests.t, which
  is running its own test DNS server in a subprocess
Sending        MANIFEST
Sending        lib/Mail/SpamAssassin/Conf.pm
Sending        lib/Mail/SpamAssassin/Dns.pm
Sending        lib/Mail/SpamAssassin/DnsResolver.pm
Sending        lib/Mail/SpamAssassin/Plugin/URIDNSBL.pm
Adding         t/dnsbl_subtests.t
Transmitting file data ......
Committed revision 921052.

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