Skip Menu |

Preferred bug tracker

Please visit the preferred bug tracker to report your issue.

This queue is for tickets about the Email-Valid CPAN distribution.

Report information
The Basics
Id: 11905
Status: resolved
Priority: 0/
Queue: Email-Valid

People
Owner: rjbs [...] cpan.org
Requestors: smylers [...] cpan.org
Cc:
AdminCc:

Bug Information
Severity: (no value)
Broken in: (no value)
Fixed in: (no value)



Subject: [PATCH] nslookup Fork Fails on Windows
Hello there. Thank you very much for writing Email::Valid -- it's an excellent module, and we make good use of it. Recently we've tried deploying it on Windows and found a problem with the code that uses the external nslookup command for the MX check. This includes the line: if (my $fh = new IO::File '-|') { Unfortunately that doesn't work on Windows. It's documented not to work in perlfork (search for "Forking pipe") and it gives this error message: '-' is not recognized as an internal or external command, operable program or batch file. perlfork suggests a work-around, and provides the function pipe_from_fork. However Uri Guttman reports that it doesn't work: http://groups.google.com/groups?threadm=x73cbvohjn.fsf@mail.sysarch.com And it doesn't seem to work for me either. I tried changing Email::Valid to use IO::Pipe instead, but that provides no way of either capturing or squashing things sent to standard error, and nslookup prints to stderr if a domain isn't found. So I've used IO::CaptureOutput instead. This seems to work (I've tested it on both Windows and FreeBSD). Please see the attached patch. Let me know if you have any questions. Cheers. Smylers
--- /usr/local/lib/perl5/site_perl/5.8.5/Email/Valid.pm Sat Aug 23 09:28:03 2003 +++ ./Valid.pm Wed Mar 16 13:57:45 2005 @@ -5,7 +5,7 @@ @NSLOOKUP_PATHS $Details $Resolver $Nslookup_Path $DNS_Method $TLD $Debug ); use Carp; -use IO::File; +use IO::CaptureOutput qw<capture_exec>; use Mail::Address; use File::Spec; @@ -142,21 +142,11 @@ return 1 if gethostbyname $host; # Check for an MX record - if (my $fh = new IO::File '-|') { - my $response = <$fh>; - print STDERR $response if $Debug; - close $fh; - $response =~ /$NSLOOKUP_PAT/io or return $self->details('mx'); - return 1; - } else { - open OLDERR, '>&STDERR' or croak "cannot dup stderr: $!"; - open STDERR, '>&STDOUT' or croak "cannot redirect stderr to stdout: $!"; - { - exec $Nslookup_Path, '-query=mx', $host; - } - open STDERR, ">&OLDERR"; - croak "unable to execute nslookup '$Nslookup_Path': $!"; - } + my $response = capture_exec $Nslookup_Path, '-query=mx', $host; + croak "unable to execute nslookup '$Nslookup_Path': exit $?" if $?; + print STDERR $response if $Debug; + $response =~ /$NSLOOKUP_PAT/io or return $self->details('mx'); + return 1; } # Purpose: Check whether a top level domain is valid for a domain.
Thanks for the patch and the info. I do not want to make IO::CaptureOutput a prereq of E::V, so I will apply this patch with some changes to make it check $^O. This should be done in or before version 0.18, which shoudl come out within the week. -- rjbs
Ok, I just added the conditional. I can cope with writing two extra lines, right? Right! Let me know if this helps, if you still care. Otherwise I will call this resolved soon. -- rjbs
Subject: Re: [rt.cpan.org #11905] [PATCH] nslookup Fork Fails on Windows
Date: Fri, 9 Jun 2006 07:14:58 +0100
To: Ricardo Signes via RT <bug-Email-Valid [...] rt.cpan.org>
From: Smylers <smylers [...] cpan.org>
Ricardo Signes via RT writes: Show quoted text
> Ok, I just added the conditional. I can cope with writing two extra > lines, right? Right!
Thanks! Somewhat amusingly my actions this morning included these, in this order: * Check feeds in Bloglines, including Cpan uploads. * Spot a new version of Email::Valid, and follow the link to find out more. * Note that yet another of the modules I use has been taken over by RJBS, and wonder if it'll reach the state where I'm only using RJBS modules. * Look at the change log to see what's new. * Be surprised to spot me credited with some change! * Find this bug on RT to see what on earth I did. * Realize from looking at the bug that I had mail from you in my inbox! Show quoted text
> Let me know if this helps, if you still care.
It looks good. We are using Email::Valid on several hundred Windows servers; I'd forgotten about this issue, so I guess we're using it with my patch applied locally. Show quoted text
> Otherwise I will call this resolved soon.
My only quibble is that I don't think IO::CaptureOutput is a module many Windows users will have installed (I hadn't heard of it until trying to fix this bug), so on first use this will still fail in a rather surprising way, one which may look to the uninitiated user like a problem in your module. I have 2 suggestions: * Put the require IO::CaptureOutput line in an eval block, and if it fails die with a user-friendly error stating that the module is required Windows and asking the user to load it. * In Makefile.PL have IO::CaptureOutput conditionally added to PREREQ_PM, using the same $^O test as in the module. Cheers. Smylers
CC: undisclosed-recipients: ;
Subject: Re: [rt.cpan.org #11905] [PATCH] nslookup Fork Fails on Windows
Date: Fri, 9 Jun 2006 07:33:51 -0400
To: "smylers [...] cpan.org via RT" <bug-Email-Valid [...] rt.cpan.org>
From: Ricardo SIGNES <rjbs [...] manxome.org>
* "smylers@cpan.org via RT" <bug-Email-Valid@rt.cpan.org> [2006-06-09T02:15:32] Show quoted text
> * Note that yet another of the modules I use has been taken over by > RJBS, and wonder if it'll reach the state where I'm only using RJBS > modules.
Dare to dream! Show quoted text
> My only quibble is that I don't think IO::CaptureOutput is a module many > Windows users will have installed (I hadn't heard of it until trying to > fix this bug), so on first use this will still fail in a rather > surprising way, one which may look to the uninitiated user like a > problem in your module.
Yes, I agree. My plan is to switch to Module::Install and improve the prereq situation, which, imo, stinks for all users. At install time there should be an explanation of what features require what modules. Probably v0.180 -- rjbs
Download signature.asc
application/pgp-signature 189b

Message body not shown because it is not plain text.