rt icon indicating copy to clipboard operation
rt copied to clipboard

Force use of IPv4 for LDAP test.

Open puck opened this issue 1 year ago • 7 comments

Net::LDAP::Server::Test binds to IPv6 by default if IPv6 is enabled, but Net::LDAP uses 'localhost' which resolves to an IPv4 address. Even when I switched the call to Net::LDAP->new() to use ip6-localhost it failed elsewhere due to RT using 127.0.0.1.

puck avatar Sep 04 '23 11:09 puck

Hmm, this makes me wonder what else currently expects localhost to be '127.0.0.1' vs. '::1'. Assuming you ran RTs tests on your IPv6 enabled system, are the LDAP tests the only ones that failed?

cbrandtbuffalo avatar Sep 05 '23 12:09 cbrandtbuffalo

I'm running the tests on a dual stack system. The LDAP tests are the only ones that fail for me without this patch.

A quick grep on 5.0-trunk shows this:

puck@dirk:~/personal/RT/rt$ grep -r "127\.0\.0\.1" * | grep -v "t/data/emails"
docs/web_deployment.pod:    spawn-fcgi -u www-data -g www-data -a 127.0.0.1 -p 9000 \
docs/web_deployment.pod:            fastcgi_pass 127.0.0.1:9000;
lib/RT/Interface/Web.pm:to handle common problems such as localhost vs 127.0.0.1
lib/RT/Interface/Web.pm:    $uri->host('127.0.0.1') if $uri->host eq 'localhost';
sbin/rt-setup-fulltext-index:    $url = 'sphinx://127.0.0.1:3312/rt'
sbin/rt-setup-fulltext-index.in:    $url = 'sphinx://127.0.0.1:3312/rt'
t/fts/indexed_sphinx.t:        url            => "sphinx://127.0.0.1:$port/rt",
t/externalauth/ldap_escaping.t:            'server'          => "127.0.0.1:$ldap_port",
t/externalauth/ldap_privileged.t:            'server'          => "127.0.0.1:$ldap_port",
t/externalauth/ldap_group.t:            'server'          => "127.0.0.1:$ldap_port",
t/externalauth/ldap.t:            'server'          => "127.0.0.1:$ldap_port",
t/externalauth/ldap.t:            'server'          => "127.0.0.1:$ldap_port",
t/externalauth/ldap_email_login.t:            'server'          => "127.0.0.1:$ldap_port",
puck@dirk:~/personal/RT/rt$ 

puck avatar Sep 05 '23 22:09 puck

I can't remember what broke when I tried using ip6-localhost, but switching to that will cause breakage on hosts which don't have IPv6 enabled.

puck avatar Sep 05 '23 23:09 puck

Oh, and given the amount of repeated code, standing up a test LDAP server feels like a good thing to abstract into a module. :)

puck avatar Sep 05 '23 23:09 puck

Oh, and given the amount of repeated code, standing up a test LDAP server feels like a good thing to abstract into a module. :)

Yeah, that's a good thought. We have various helper modules for that reason, like lib/RT/Test.pm, lib/RT/Test/Email.pm, etc. Did you want to take a shot at updating your branch to move the that bit of LDAP server code to a module?

cbrandtbuffalo avatar Sep 08 '23 13:09 cbrandtbuffalo

That's what I get for opening my mouth. ;) I'm working on a new lib/RT/Test/LDAP.pm file. Fortunately this is trivial to run tests for!

puck avatar Sep 15 '23 03:09 puck

I've pushed a commit which creates RT::Test::LDAP and changes all the tests for LDAP (ExternalAuth and LDAPImport) to use it.

puck avatar Sep 22 '23 13:09 puck