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 2005/06/21 03:07:22 UTC
[Bug 4412] New: DnsResolver.pm/connect_sock() fails on IPv6 resolver address
http://bugzilla.spamassassin.org/show_bug.cgi?id=4412
Summary: DnsResolver.pm/connect_sock() fails on IPv6 resolver
address
Product: Spamassassin
Version: SVN Trunk (Latest Devel Version)
Platform: PC
OS/Version: FreeBSD
Status: NEW
Severity: normal
Priority: P5
Component: Libraries
AssignedTo: dev@spamassassin.apache.org
ReportedBy: Mark.Martinec@ijs.si
The code section in DnsResolver.pm/connect_sock:
$self->{sock} = $sock;
$self->{dest} = sockaddr_in($self->{res}->{port},
inet_aton($self->{res}->{nameservers}[0]));
fails because $self->{res}->{nameservers}[0] is a string "::1"
and inet_aton does not recognize IPv6 syntax.
environment: spamassassin_20050620223535 (SVN),
Net::DNS 0.51, IPv6-enabled OS, module IO::Socket::INET6 installed,
/etc/resolv.conf contains entry: nameserver ::1
The problem is easily repeatable:
$ spamassassin -tD
[50765] dbg: logger: adding facilities: all
[50765] dbg: logger: logging level is DBG
[50765] dbg: generic: SpamAssassin version 3.1.0-pre2-r191258
...
[50765] dbg: dns: is Net::DNS::Resolver available? yes
[50765] dbg: dns: Net::DNS version: 0.51
[50765] warn: Use of uninitialized value in subroutine entry
at /usr/local/lib/perl5/5.8.6/mach/Socket.pm line 373.
Bad arg length for Socket::pack_sockaddr_in, length is 0,
should be 4
at /usr/local/lib/perl5/5.8.6/mach/Socket.pm line 373.
It is interesting that this used to work (with IPv6 resolver)
with somewhat older SVN version of SA (spamassassin_20050526104220).
Note that the current Net::DNS 0.51 does support IPv6 resolvers
when module IO::Socket::INET6 is installed. Seems to be the
SA-supplied wrapper/replacement routines in DnsResolver.pm
try to over-optimize what is available and working in Net::DNS.
A workaround is to remove the "nameserver ::1" entry
from /etc/resolv.conf and provide only IPv4 addresses
of resolvers. Unfortunately this also affects other
applications on the host.
Regards
Mark
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
[Bug 4412] DnsResolver.pm/connect_sock() fails on IPv6 resolver address
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4412
jm@jmason.org changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #3074 is|0 |1
obsolete| |
------- Additional Comments From jm@jmason.org 2005-08-16 20:04 -------
Created an attachment (id=3078)
--> (http://bugzilla.spamassassin.org/attachment.cgi?id=3078&action=view)
update of 3074
thanks John! I've made a couple of changes to that with this rev:
- used constants instead of a variable to indicate module presence,
that's more "the SA way"
- added an entry to lib/Mail/SpamAssassin/Util/DependencyInfo.pm so
that it'll be flagged at Makefile.PL time
- added entry about the module to INSTALL
- added some code to produce an explanatory warning for the situation where
someone's using ipv6 systemwide, has an ipv6 addr in resolv.conf, but
doesn't have IO::Socket::INET6 installed.
The latter situation now looks like this:
: jm 116...; RES_NAMESERVERS=::1 ./spamassassin.raw -D --lint
[31283] dbg: logger: adding facilities: all
[31283] dbg: logger: logging level is DBG
[31283] dbg: generic: SpamAssassin version 3.2.0-r232569
[31283] dbg: config: score set 0 chosen.
[31283] dbg: util: running in taint mode? no
[31283] dbg: dns: is Net::DNS::Resolver available? yes
[31283] dbg: dns: Net::DNS version: 0.48
IO::Socket::INET6 module is required to use IPv6 nameservers such as '::1':
IO::Socket::INET: Bad hostname '::1'
Note to reviewers: that die() call in _ipv6_ns_warning() is required instead of
a warn() -- without that, it'll loop, producing the warning infinitely until
CTRL-C'd.
I think this is reviewable for 3.1.0. I'd be OK with leaving it
for 3.1.1, though.
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
[Bug 4412] DnsResolver.pm/connect_sock() fails on IPv6 resolver address
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4412
------- Additional Comments From Mark.Martinec@ijs.si 2005-06-21 16:06 -------
*** Bug 4417 has been marked as a duplicate of this bug. ***
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
[Bug 4412] [review] DnsResolver.pm/connect_sock() fails on IPv6 resolver address
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4412
duncf@debian.org changed:
What |Removed |Added
----------------------------------------------------------------------------
Status Whiteboard|needs 1 vote |ready to commit
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
[Bug 4412] [review] DnsResolver.pm/connect_sock() fails on IPv6 resolver address
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4412
------- Additional Comments From sidney@sidney.com 2005-08-18 17:10 -------
I believe Net::DNS went through some iterations of fixes for AF_INET6 bare word
problem. See if you can make sense of the code around the use of AF_INET6 in
Net::DNS::Resolver::Base.pm
If I remember correctly, there was some conditional expression that had to be
put in place everywhere that AF_INET6 appeared.
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
[Bug 4412] DnsResolver.pm/connect_sock() fails on IPv6 resolver address
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4412
jm@jmason.org changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #3088 is|0 |1
obsolete| |
------- Additional Comments From jm@jmason.org 2005-08-19 19:49 -------
Created an attachment (id=3090)
--> (http://bugzilla.spamassassin.org/attachment.cgi?id=3090&action=view)
redo of 3088
OK, this looks a lot more promising -- the buildbots are chugging away without
a failure so far: http://spamassassin.zones.apache.org:8010/
this patch is from svn trunk from those 2 checkins, and should apply to b3_1_0.
assuming the Buildbots pass, please vote...
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
[Bug 4412] [review] DnsResolver.pm/connect_sock() fails on IPv6 resolver address
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4412
------- Additional Comments From jm@jmason.org 2005-08-18 12:32 -------
actually, Mark has a CLA on-file -- updating the bz list to match that.
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
[Bug 4412] [review] DnsResolver.pm/connect_sock() fails on IPv6 resolver address
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4412
------- Additional Comments From sidney@sidney.com 2005-08-18 16:38 -------
> The main point is in reporting the $! when
> IO::Socket::INET*->new fails and the $! is not some
> expected status like EIADDRINUSE
That makes sense. WHat I was trying to avoid in not checking for EIADDRINUSE is
having it all fail due to some other relatively benign error that is like
EIADDRINUSE.
Which is more correct would depend on whether you are confident that anything
other than EIADDRINUSE and the INET6 problem is a hard error that will persist
across further attempts on different ports, or whether you aren't sure if there
might be such temporary errors other than EIADDRINUSE.
Since I don't know much abo9ut the various error status codes when trying to
open a socket, I chose the latter. However, the difficulty you had in debugging
this certainly demonstrates the benefits of your approach when an unforseen hard
error does happen.
I'll leave the decision on which way to go with this to Justin or other
committers who want to express an opinion about it.
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
[Bug 4412] [review] DnsResolver.pm/connect_sock() fails on IPv6 resolver address
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4412
------- Additional Comments From jm@jmason.org 2005-08-17 12:33 -------
btw I'd really prefer not to use the code from Net::DNS::Resolver -- look at it,
it's terrifying! let's just use IO::Socket::INET6 and keep it simple.
having said that, yes, I meant to fix the bgsend case and it looks like I
forgot. it's harmless -- IO::Socket::INET6 and ::INET do the right thing afaik
-- but it's unclear.
basically, with UDP sockets you have two choices:
1. specify the dest endpoint in the IO::Socket::INET constructor, or
2. specify it at send() time
that works for IPv4, and #2 is what we were using in Resolver. However, for
IPv6 with IO::Socket::INET6, only #1 works, #2 seems to be no longer supported.
therefore we have to change our code to use the #1 approach instead of #2.
Since we only ever used 1 endpoint anyway, this is no biggie for us anyway.
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
[Bug 4412] [review] DnsResolver.pm/connect_sock() fails on IPv6 resolver address
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4412
------- Additional Comments From jm@jmason.org 2005-08-24 20:42 -------
(btw to follow on from my last comment: all tests passed, the patch works. and
it still needs 1 more vote ;)
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
[Bug 4412] DnsResolver.pm/connect_sock() fails on IPv6 resolver address
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4412
------- Additional Comments From sidney@sidney.com 2005-06-20 23:14 -------
I looked at the code in sub send_udp in Net::DNS::Resolver::Base.pm
Probably I would do it the way it is now done there in the latest version.
I'll be working on finishing my final term paper of the semester for the next
four or five days and will not get to this before that's finished. If anyone
else wants to pick up on this, patches will be gratefully appreciated. Do make
sure that you look at Net::DNS in case there is anything in there that is
differnt from what you have in mind. The latest svn version of Net::DNS can be
pulled down using
svn co -r 386 http://www.net-dns.org/svn/net-dns/trunk
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
[Bug 4412] [review] DnsResolver.pm/connect_sock() fails on IPv6 resolver address
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4412
------- Additional Comments From maddenj+spamassassin@skynet.ie 2005-08-19 02:41 -------
Subject: Re: [review] DnsResolver.pm/connect_sock() fails on IPv6 resolver address
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On (18/08/05 17:16), bugzilla-daemon@bugzilla.spamassassin.org didst pronounce:
> it helps a little -- I think trial and error will be required.
>
Could you not redefine it if IO::Socket::INET6 isn't available?
if(!HAS_SOCKET_INET6) {
use constant AF_INET6 => 0;
}
Or something like that? AF_INET6 depends on HAS_SOCKET_INET6 being set,
and if it's not set, AF_INET6 is never needed.
- --
Chat ya later,
John.
- --
BOFH excuse #159: Stubborn processes
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (GNU/Linux)
iD8DBQFDBapcQBw+ZtKOvTIRAlJXAJ9cF+66In9/MaiYzFTx0oxnGviTpwCeLdf3
pTePW0lK+Fsvgq5B6fBtpLQ=
=uGLf
-----END PGP SIGNATURE-----
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
[Bug 4412] [review] DnsResolver.pm/connect_sock() fails on IPv6 resolver address
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4412
------- Additional Comments From jm@jmason.org 2005-08-18 17:06 -------
the drawing board calls. ;)
http://spamassassin.zones.apache.org:8010/t-red-hat-73/builds/20/compile/0
http://spamassassin.zones.apache.org:8010/t-sol10-561/builds/23/compile/0
The error was:
version.h.pl: version.h.pl: version.h.pl: version.h.pl: Bareword "AF_INET6" not
allowed while "strict subs" in use at ../lib/Mail/SpamAssassin/DnsResolver.pm
line 163.
BEGIN not safe after errors--compilation aborted at
../lib/Mail/SpamAssassin/DnsResolver.pm line 476.
Compilation failed in require at ../lib/Mail/SpamAssassin.pm line 75.
BEGIN failed--compilation aborted at ../lib/Mail/SpamAssassin.pm line 75.
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
[Bug 4412] [review] DnsResolver.pm/connect_sock() fails on IPv6 resolver address
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4412
------- Additional Comments From automasschecker@jmason.org 2005-08-17 16:23 -------
Subject: Re: [review] DnsResolver.pm/connect_sock() fails on IPv6 resolver address
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
> I see that my $data = $pkt->data; still seems to be unnecessary, with $data
> never being referenced. Can it be removed as part of the patch?
yeah, trivial, we can do that at checkin ;)
> I have a question about something I noticed while checking over the code
> for the patch which exceeds my knowledge of perl. In sub connect_sock
> there is a line
> $self->{sock}->close() if $self->{sock};
>
> In sub finish_socket I see
>
> if ($self->{sock}) {
> $self->{sock}->close();
> delete $self->{sock};
> }
>
> Is the delete necessary in connect_sock too?
No, it's not necessary -- when $self->{sock} is set later on in the
method, it GC's the old $self->{sock} ref first, in other words doing
what the delete does, implicitly. It'd possibly be nice to make
it explicit, but really, no big deal.
> Should connect_sock just call
> finish_socket there instead of {sock}->close?
It could do -- I'd go either way. On one hand, it'd be nice to share the
code; OTOH, I've often run into situations where a public "close" API
needs to do slightly different things compared to a "open, closing if
already open first" API. I'm fine leaving it the way it is.
- --j.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.5 (GNU/Linux)
Comment: Exmh CVS
iD8DBQFDA8YyMJF5cimLx9ARAj6+AKCVrF9Hqs1U0tWLeIYc+hs2Ywb8lACfRuu8
dKXZWhbQHsbx4CuVw/ZoMUM=
=AWmh
-----END PGP SIGNATURE-----
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
[Bug 4412] DnsResolver.pm/connect_sock() fails on IPv6 resolver address
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4412
------- Additional Comments From jgmyers@proofpoint.com 2005-06-20 22:30 -------
::1 is an IP address, the IPv6 address similar to the IPv4 address 127.0.0.1.
See RFC 2373 sections 2.2.2 and 2.5.3.
To create a socket, you should probably use IO::Socket::INET6 if that module is
installed, falling back to IO::Socket::INET if not. I would recommend
surrounding the nameserver's IP address with square brackets, passing that as
the the PeerAddr argument to IO::Socket::INET(6)?::new. The call to
sockaddr_in() I would replace with a call to the socket's peerhost() method.
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
[Bug 4412] [review] DnsResolver.pm/connect_sock() fails on IPv6 resolver address
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4412
------- Additional Comments From sidney@sidney.com 2005-08-17 09:47 -------
The patch passes make test on my system, but I don't understand what is going on
with this part of the patch that removes two lines of code
- $self->{dest} = sockaddr_in($self->{res}->{port},
- inet_aton($self->{res}->{nameservers}[0]));
I see that $self->{dest} is used later in sub bgsend in the following snippet:
my $data = $pkt->data;
my $dest = $self->{dest};
$self->connect_sock() if !$self->{sock};
if (!$self->{sock}->send ($pkt->data, 0, $self->{dest})) {
warn "dns: sendto() failed: $@";
return;
}
And then $data and $dest are never used, which is a separate issue.
The code in Net::DNS::Resolver::Base.pm version 0.53 in sub send_udp that
computes the equivalent of $self->{dest} here distinguishes between IPv6 and
IPv4 doing something like the following to compute what it names $dst_sockaddr:
In the IPv4 case it looks like
$dst_sockaddr = sockaddr_in($dstport, inet_aton($ns_address));
In the IPv6 case it looks like
# we can use getaddrinfo
no strict 'subs'; # Because of the eval statement in the BEGIN
# AI_NUMERICHOST is not available at compile time.
# The AI_NUMERICHOST surpresses lookups.
my $old_wflag = $^W; #circumvent perl -w warnings about 'udp'
$^W = 0;
my @res = getaddrinfo($ns_address, $dstport, AF_UNSPEC, SOCK_DGRAM,
0, AI_NUMERICHOST);
$^W=$old_wflag ;
use strict 'subs';
my ($sockfamily, $socktype_tmp,
$proto_tmp, $dst_sockaddr, $canonname_tmp) = @res;
So do we need equivalent code to compute $self->{dest}? Why were those lines
just removed in the patch if $self->{dest} is later used in sub bgsend?
I don't have time right now to figure out a test case to demonstrate bgsend
breaking with this patch or to figure out why it doesn't break, but this doesn't
look to me like it could be correct.
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
[Bug 4412] [review] DnsResolver.pm/connect_sock() fails on IPv6 resolver address
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4412
------- Additional Comments From sidney@sidney.com 2005-08-16 20:30 -------
This should be tested with Net::DNS version > 0.48. Version fifty-something
added support for IPv6 that may give this different behavior. I don't know that
for sure, but I want to raise the flag to make sure this gets reviewed using the
latest Net::DNS. I'll try to get to that tonight.
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
[Bug 4412] [review] DnsResolver.pm/connect_sock() fails on IPv6 resolver address
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4412
------- Additional Comments From sidney@sidney.com 2005-08-17 12:16 -------
Yes, the code that was removed is incompatible with IPv6, but it does look like
it is needed for bgsend and so cannot simply be removed. It would be nice to
figure out a test case that demonstrates what removing it breaks, as make test
does work with this patch, at least on my IPv4 only system. Without
understanding this well enough to show such a test case I'm wary of just putting
in some code that looks like it should work that is based on the snippet from
Net::DNS that I posted.
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
[Bug 4412] DnsResolver.pm/connect_sock() fails on IPv6 resolver address
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4412
jm@jmason.org changed:
What |Removed |Added
----------------------------------------------------------------------------
Summary|[review] |DnsResolver.pm/connect_sock(
|DnsResolver.pm/connect_sock(|) fails on IPv6 resolver
|) fails on IPv6 resolver |address
|address |
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
[Bug 4412] [review] DnsResolver.pm/connect_sock() fails on IPv6 resolver address
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4412
sidney@sidney.com changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|RESOLVED |REOPENED
Resolution|FIXED |
------- Additional Comments From sidney@sidney.com 2005-08-18 12:09 -------
I must have hit a control key by accident when typing my last comment. I didn't
intend to mark this bug fixed. Reopening.
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
[Bug 4412] [review] DnsResolver.pm/connect_sock() fails on IPv6 resolver address
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4412
------- Additional Comments From jm@jmason.org 2005-08-18 16:45 -------
ok, patch 3088 is in trunk as r233398.
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
[Bug 4412] [review] DnsResolver.pm/connect_sock() fails on IPv6 resolver address
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4412
jm@jmason.org changed:
What |Removed |Added
----------------------------------------------------------------------------
Summary|DnsResolver.pm/connect_sock(|[review]
|) fails on IPv6 resolver |DnsResolver.pm/connect_sock(
|address |) fails on IPv6 resolver
| |address
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
[Bug 4412] DnsResolver.pm/connect_sock() fails on IPv6 resolver address
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4412
------- Additional Comments From jm@jmason.org 2005-08-04 17:17 -------
should this block 3.1.0? It's now documented in the FAQ so imo no, but it'd be
nice to fix.
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
[Bug 4412] [review] DnsResolver.pm/connect_sock() fails on IPv6 resolver address
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4412
------- Additional Comments From Mark.Martinec@ijs.si 2005-08-18 10:53 -------
Created an attachment (id=3087)
--> (http://bugzilla.spamassassin.org/attachment.cgi?id=3087&action=view)
better error handling and work around IO::Socket::INET6 family quirk
The patch applies on top of Justin's rev3 patch and irons out
some remaining IPv4/IPv6 issues in DnsResolver.pm (it didn't work
out of the box):
- properly report error status on unexpected failures to create socket;
(previously undef $socket went on undetected and caused subsequent
failure elsewhere);
- avoid 65000 attempts on other than EADDRINUSE failure;
- work around a quirk in IO::Socket::INET6 where a creation of a
socket fails because LocalAddr is auto-asssigned but happens to
belong to other address family; author has been notified; either
specifying Domain explicitly or providing LocalAddr of the correct
family explicitly avoids the problem - I chose to provide the family;
Now it works in my environment, with or without IO::Socket::INET6
(missing module properly reported when appropriate),
with IPv4 or IPv6 resolver address.
(you may want to adjust the coding style)
Mark
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
[Bug 4412] DnsResolver.pm/connect_sock() fails on IPv6 resolver address
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4412
------- Additional Comments From sidney@sidney.com 2005-08-19 21:06 -------
+1
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
[Bug 4412] [review] DnsResolver.pm/connect_sock() fails on IPv6 resolver address
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4412
jm@jmason.org changed:
What |Removed |Added
----------------------------------------------------------------------------
Summary|DnsResolver.pm/connect_sock(|[review]
|) fails on IPv6 resolver |DnsResolver.pm/connect_sock(
|address |) fails on IPv6 resolver
| |address
Status Whiteboard| |needs 1 vote
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
[Bug 4412] DnsResolver.pm/connect_sock() fails on IPv6 resolver address
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4412
sidney@sidney.com changed:
What |Removed |Added
----------------------------------------------------------------------------
Target Milestone|Undefined |3.1.0
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
[Bug 4412] [review] DnsResolver.pm/connect_sock() fails on IPv6 resolver address
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4412
------- Additional Comments From sidney@sidney.com 2005-10-16 21:25 -------
I'm adding a commment here to document a change related to commment 24 that I
just checked in as revision 325836 for bug 4619 in case a problem leads back to
this bug report.
That change contains a different fix for this bug's comment 24 that does not
require using the AF_INET6 macro and therefore doesn't neeed the various tests
for AF_INET6 being defined. The problem brought up in comment 24 is that on some
platforms creating a socket using AF_UNSPEC doesn't work if the source and
destination addresses are in different domains, with the symptom being an EINVAL
error on the send. Instead of replacing AF_UNSPEC with AF_INET6 or AF_INET, as
was done here, the new fix replaces the LocalAddr with "::" or "0.0.0.0" to
match the PeerAddr family.
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
[Bug 4412] [review] DnsResolver.pm/connect_sock() fails on IPv6 resolver address
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4412
------- Additional Comments From lwilton@earthlink.net 2005-08-17 11:51 -------
Subject: Re: [review] DnsResolver.pm/connect_sock() fails on IPv6 resolver address
> The patch passes make test on my system, but I don't understand what is
going on
> with this part of the patch that removes two lines of code
>
> - $self->{dest} = sockaddr_in($self->{res}->{port},
> - inet_aton($self->{res}->{nameservers}[0]));
The OP mentioned that inet_aton as coded was incompatable with IPV6. I
suspect that may be related to why the code was removed. Perhaps the
correct solution is similar to the code you posted for IPV6, and a test to
decide which method to use.
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
[Bug 4412] DnsResolver.pm/connect_sock() fails on IPv6 resolver address
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4412
------- Additional Comments From sidney@sidney.com 2005-06-20 21:25 -------
I've been working on DnsResolver, but I'm not familiar with IPv6. Can you point
me to some documentation? What does the "::1" mean in the nameservers array? How
should it be used to create a socket?
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
[Bug 4412] [review] DnsResolver.pm/connect_sock() fails on IPv6 resolver address
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4412
------- Additional Comments From jm@jmason.org 2005-08-18 16:41 -------
I initially sided with Sidney, but Mark's statements all make sense to me. We
have often had a bit of a thing for premature workarounds; going out of our way
to fix stuff that we don't know is actually going to break. So let's go with
the improved error reporting, and only later see if there's a problem left that
we need to worry about...
One thing I'm worried about btw -- the use of AF_INET6 as a bare word. There's a
possibility that some platform may not have that defined -- but I cannot think
what we can do about that without tying ourselves in knots. Again, let's see if
there are any reports of that being a problem, before trying to fix it. (I've
left in an explanatory comment just in case.)
I'll repackage that patch as against SVN trunk and let's get some votes.
I'll also check it into trunk so buildbot can see how we do on the AF_INET6
bareword issue.
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
[Bug 4412] [review] DnsResolver.pm/connect_sock() fails on IPv6 resolver address
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4412
------- Additional Comments From sidney@sidney.com 2005-08-17 15:51 -------
I'm glad we can get away with code that is so much simpler than what Net::DNS
has to do.
I see that my $data = $pkt->data; still seems to be unnecessary, with $data
never being referenced. Can it be removed as part of the patch?
I have a question about something I noticed while checking over the code for the
patch which exceeds my knowledge of perl. In sub connect_sock there is a line
$self->{sock}->close() if $self->{sock};
In sub finish_socket I see
if ($self->{sock}) {
$self->{sock}->close();
delete $self->{sock};
}
Is the delete necessary in connect_sock too? Should connect_sock just call
finish_socket there instead of {sock}->close?
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
[Bug 4412] [review] DnsResolver.pm/connect_sock() fails on IPv6 resolver address
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4412
------- Additional Comments From jm@jmason.org 2005-08-18 17:16 -------
it helps a little -- I think trial and error will be required.
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
[Bug 4412] DnsResolver.pm/connect_sock() fails on IPv6 resolver address
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4412
------- Additional Comments From maddenj+spamassassin@skynet.ie 2005-08-11 13:47 -------
Created an attachment (id=3074)
--> (http://bugzilla.spamassassin.org/attachment.cgi?id=3074&action=view)
IPV6 support
I'm not sure if this will fully fix this bug. One gotcha that I can pick out is
if the system is configured to use ipv6, has an ipv6 address in
/etc/resolv.conf but doesn't have IO::Socket::INET6 installed. Personally, I
don't really see the point, but it's possible, and this patch won't help in
that situation.
It's been a while since I hacked Perl, so be gentle!! :)
BTW, I have spamd listening on ::1 on my system, and spamc talking to it, and
all seems to be working ok. I've to clean up the patches, but I'll upload them
in the next few days.
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
[Bug 4412] [review] DnsResolver.pm/connect_sock() fails on IPv6 resolver address
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4412
------- Additional Comments From Mark.Martinec@ijs.si 2005-08-18 15:42 -------
> Also, what failures did you see that you addressed with the change
> "- avoid 65000 attempts on other than EADDRINUSE failure;"
Sorry for being too sketchy...
> I originally did not check for EIADDRINUSE because I thought that
> if it is something else and it works on trying again on another port,
> then why not just keep on trying? If there is some common failure that
> we would want to die immediately on, then we can check for that.
That is what I had in mind. The reasoning is that if something
unexpected happens, it is better to be reported as soon as possible
and with as much information as possible (the $!), instead of being
possibly masked by further attempts. An imaginary scenario to be avoided
is that some day someone would report that a DNS connect takes enormous
amount of time or allocates much system resources, only to be discovered
that it was due to original problem not being noticed and reported.
The background story is what happened when I initially tried that
code with rev3 patch (I had IPv4 address in resolv.conf, and the
module IO::Socket::INET6 was installed): the 'make test' of SA was
very slow and failing:
PERL_DL_NONLAZY=1 /usr/local/bin/perl "-MExtUtils::Command::MM" "-e"
"test_harness(0, 'blib/lib', 'blib/arch')" t/*.tt/basic_lint ....
Can't use an undefined value as a symbol reference
at ../blib/lib/Mail/SpamAssassin/DnsResolver.pm line 428.
It turned out the "Can't use an undefined value as a symbol reference"
was caused by fileno(undef), because the @fhlist in sub fhs_to_vec
had one element which was undef. Tracking this back brought me to
sub connect_sock, where the IO::Socket::INET6->new was failing with
EINVAL because of the wrong address family implicitly chosen.
Unfortunately it didn't report the failure, and wasted the whole round
of 64k attempts to not do anything useful but return undef in $sock.
WHICH MADE ME FIX THE ERROR REPORTING FIRST. Ignoring status, or not
reporting the full error status is always bad, at least in the long run,
when one has to deal with hordes of users, and remote troubleshooting
is very hard to do.
> And how does your patch avoid that?
By calling die with $! in the message.
> I only see that it checks for EIADDRINUSE and if that's not the error,
> it calls _ipv6_ns_warning which only dies if the error is a problem
> with IPv6 nameserver
... AND THEN if _ipv6_ns_warning does not report and dies by itself,
then the: die "Error creating a DNS resolver socket: $errno";
does this instead.
> The changes I see in the patch are:
>
> 1) Add a Domain argument to the IO::Socket constructor call set to:
> If PeerAddr is in IPv6 family and we have IO::Socket::INET6 set it
> to AF_INET6, else set it to AF_INET
True. Perhaps the Domain attribute can even be left unspecified
if HAS_SOCKET_INET6 is false (just in case if comeone does not even
have the AF_INET6 constant).
> 2) In the loop that looks for an unused port, skip the call to
> _ipv6_ns_warning if $! is EIADDRINUSE because that's the error
> we intend to loop over.
That was secondary. The main point is in reporting the $!
when IO::Socket::INET*->new fails and the $! is not some
expected status like EIADDRINUSE, which we know how to deal with.
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
[Bug 4412] DnsResolver.pm/connect_sock() fails on IPv6 resolver address
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4412
------- Additional Comments From jm@jmason.org 2005-06-20 22:21 -------
Sidney -- iirc, "::1" is equivalent to "127.0.0.1" in IPv6 formatting. (there's
a whole host of abbreviation formats in ipv6 btw.) there's a little doco on
IPv6 hostname formatting in lib/Mail/SA/Constants.pm I think.
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
[Bug 4412] [review] DnsResolver.pm/connect_sock() fails on IPv6 resolver address
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4412
------- Additional Comments From sidney@sidney.com 2005-08-18 17:15 -------
Now I remember. Every use of AF_INET6 was replaced with AF_INET6()
Does that help? The reason for doing that exceeds my current knowledge of perl.
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
[Bug 4412] [review] DnsResolver.pm/connect_sock() fails on IPv6 resolver address
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4412
------- Additional Comments From maddenj+spamassassin@skynet.ie 2005-08-18 02:07 -------
All seems ok with my tests using IPv6 here.
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
[Bug 4412] [review] DnsResolver.pm/connect_sock() fails on IPv6 resolver address
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4412
------- Additional Comments From sidney@sidney.com 2005-08-17 16:35 -------
+1, with the caveat that I don't have a system running IPv6 to test it with.
Runs fine on my tests with just IPv4.
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
[Bug 4412] [review] DnsResolver.pm/connect_sock() fails on IPv6 resolver address
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4412
duncf@debian.org changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|REOPENED |RESOLVED
Resolution| |FIXED
------- Additional Comments From duncf@debian.org 2005-08-27 18:01 -------
Committed revision 261907.
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
[Bug 4412] [review] DnsResolver.pm/connect_sock() fails on IPv6 resolver address
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4412
jm@jmason.org changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #3085 is|0 |1
obsolete| |
Attachment #3087 is|0 |1
obsolete| |
------- Additional Comments From jm@jmason.org 2005-08-18 16:44 -------
Created an attachment (id=3088)
--> (http://bugzilla.spamassassin.org/attachment.cgi?id=3088&action=view)
merge of 3085 and 3087
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
[Bug 4412] DnsResolver.pm/connect_sock() fails on IPv6 resolver address
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4412
sidney@sidney.com changed:
What |Removed |Added
----------------------------------------------------------------------------
OS/Version|FreeBSD |All
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
[Bug 4412] DnsResolver.pm/connect_sock() fails on IPv6 resolver address
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4412
------- Additional Comments From jm@jmason.org 2005-06-20 23:27 -------
btw this is NOT a mass-check blocker (afaik).
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
[Bug 4412] DnsResolver.pm/connect_sock() fails on IPv6 resolver address
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4412
------- Additional Comments From jm@jmason.org 2005-08-19 19:45 -------
easiest thing that was most likely to work, was to use eval 'AF_INET6'. that's
now in svn.
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
[Bug 4412] [review] DnsResolver.pm/connect_sock() fails on IPv6 resolver address
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4412
------- Additional Comments From duncf@debian.org 2005-08-27 17:41 -------
+1
jm's on vacation, so I'll apply this shortly
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
[Bug 4412] [review] DnsResolver.pm/connect_sock() fails on IPv6 resolver address
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4412
jm@jmason.org changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #3078 is|0 |1
obsolete| |
------- Additional Comments From jm@jmason.org 2005-08-17 13:45 -------
Created an attachment (id=3085)
--> (http://bugzilla.spamassassin.org/attachment.cgi?id=3085&action=view)
rev3
easy fix; just remove the optional 'dest' arg from the send() call.
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
[Bug 4412] [review] DnsResolver.pm/connect_sock() fails on IPv6 resolver address
Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4412
sidney@sidney.com changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |RESOLVED
Resolution| |FIXED
------- Additional Comments From sidney@sidney.com 2005-08-18 12:06 -------
Mark, thanks for the corrections. Can you please submit a CLA (
http://www.apache.org/licenses/#clas ) to make it easier to use your patch?
Also, what failures did you see that you addressed with the change
"- avoid 65000 attempts on other than EADDRINUSE failure;"
And how does your patch avoid that? I only see that it checks for EIADDRINUSE
and if that's not the error, it calls _ipv6_ns_warning which only dies if the
error is a problem with IPv6 nameserver.
I originally did not check for EIADDRINUSE because I thought that if it is
something else and it works on trying again on another port, then why not just
keep on trying? If there is some common failure that we would want to die
immediately on, then we can check for that.
Justin, aside from CLA issues, this is simple enough that you can rewrite Mark's
patch from scratch and may want to just because of style issues, perhaps wanting
something other than die in case of error, etc.
The changes I see in the patch are:
1) Add a Domain argument to the IO::Socket constructor call set to: If PeerAddr
is in IPv6 family and we have IO::Socket::INET6 set it to AF_INET6, else set it
to AF_INET
2) In the loop that looks for an unused port, skip the call to _ipv6_ns_warning
if $! is EIADDRINUSE because that's the error we intend to loop over.
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.