proxinstall: avoid open-coding FQDN sanity check

.. by moving it into its own subroutine. Makes the whole thing quite a
bit neater and easier to maintain.

No functional changes.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>

diff --git a/Proxmox/Sys/Net.pm b/Proxmox/Sys/Net.pm
index ed83fb0f48caf10d124310efec421d2e50b3e405..3e01d370e39629c6091bf095cfac01a42d6e47f5 100644 (file)
--- a/Proxmox/Sys/Net.pm
+++ b/Proxmox/Sys/Net.pm
@@ -4,7 +4,7 @@ use strict;
 use warnings;
 
 use base qw(Exporter);
-our @EXPORT_OK = qw(parse_ip_address parse_ip_mask);
+our @EXPORT_OK = qw(parse_ip_address parse_ip_mask parse_fqdn);
 
 our $HOSTNAME_RE = "(?:[a-zA-Z0-9](?:[a-zA-Z0-9\-]*[a-zA-Z0-9])?)";
 our $FQDN_RE = "(?:${HOSTNAME_RE}\.)*${HOSTNAME_RE}";
@@ -214,4 +214,24 @@ sub get_dhcp_fqdn : prototype() {
     return $name if defined($name) && $name =~ m/^([^\.]+)(?:\.(?:\S+))?$/;
 }
 
+# Follows the rules as laid out by proxmox_installer_common::utils::Fqdn
+sub parse_fqdn : prototype($) {
+    my ($text) = @_;
+
+    die "FQDN cannot be empty\n"
+       if !$text || length($text) == 0;
+
+    die "Purely numeric hostnames are not allowed\n"
+       if $text =~ /^[0-9]+(?:\.|$)/;
+
+    die "FQDN must only consist of alphanumeric characters and dashes\n"
+       if $text !~ m/^${Proxmox::Sys::Net::FQDN_RE}$/;
+
+    if ($text =~ m/^([^\.]+)\.(\S+)$/) {
+       return ($1, $2);
+    }
+
+    die "Hostname does not look like a fully qualified domain name\n";
+}
+
 1;

diff --git a/proxinstall b/proxinstall
index d3d5bfdd0ff42952a78b97fccd100b2a0cb5356b..351b240bcaa5c728bb837b4793457a4202a5afc3 100755 (executable)
--- a/proxinstall
+++ b/proxinstall
@@ -457,24 +457,16 @@ sub create_ipconf_view {
        $text =~ s/^\s+//;
        $text =~ s/\s+$//;
 
-       # Debian does not support purely numeric hostnames
-       if ($text && $text =~ /^[0-9]+(?:\.|$)/) {
-           Proxmox::UI::message("Purely numeric hostnames are not allowed.");
-           $hostentry->grab_focus();
-           return;
-       }
+       my ($hostname, $domainname) = eval { Proxmox::Sys::Net::parse_fqdn($text) };
+       my $err = $@;
 
-       if ($text
-           && $text =~ m/^${Proxmox::Sys::Net::FQDN_RE}$/
-           && $text !~ m/.example.invalid$/
-           && $text =~ m/^([^\.]+)\.(\S+)$/
-       ) {
-           Proxmox::Install::Config::set_hostname($1);
-           Proxmox::Install::Config::set_domain($2);
-       } else {
-           Proxmox::UI::message("Hostname does not look like a fully qualified domain name.");
+       if ($err || $text =~ m/.example.invalid$/) {
+           Proxmox::UI::message($err // 'Hostname does not look like a valid fully qualified domain name');
            $hostentry->grab_focus();
            return;
+       } else {
+           Proxmox::Install::Config::set_hostname($hostname);
+           Proxmox::Install::Config::set_domain($domainname);
        }
 
        # verify ip address

diff --git a/test/Makefile b/test/Makefile
index ae80a943c4eb37f527d99e4136047bef3d0a3c85..504637fc4f38161c66b8a8bc9b58bf877f4c8133 100644 (file)
--- a/test/Makefile
+++ b/test/Makefile
@@ -3,7 +3,7 @@ all:
 export PERLLIB=..
 
 .PHONY: check
-check: test-zfs-arc-max test-run-command
+check: test-zfs-arc-max test-run-command test-parse-fqdn
 
 .PHONY: test-zfs-arc-max
 test-zfs-arc-max:
@@ -11,3 +11,7 @@ test-zfs-arc-max:
 
 test-run-command:
        ./run-command.pl
+
+.PHONY: test-parse-fqdn
+test-parse-fqdn:
+       ./parse-fqdn.pl

diff --git a/test/parse-fqdn.pl b/test/parse-fqdn.pl
new file mode 100755 (executable)
index 0000000..1ffb300
--- /dev/null
+++ b/test/parse-fqdn.pl
@@ -0,0 +1,50 @@
+#!/usr/bin/env perl
+
+use strict;
+use warnings;
+
+use Test::More;
+
+use Proxmox::Sys::Net qw(parse_fqdn);
+
+# some constants just to avoid repeating ourselves
+use constant ERR_INVALID => "Hostname does not look like a fully qualified domain name\n";
+use constant ERR_ALPHANUM => "FQDN must only consist of alphanumeric characters and dashes\n";
+use constant ERR_NUMERIC => "Purely numeric hostnames are not allowed\n";
+use constant ERR_TOOLONG => "FQDN too long\n";
+use constant ERR_EMPTY => "FQDN cannot be empty\n";
+
+sub is_parsed {
+    my ($fqdn, $expected) = @_;
+
+    my @parsed = parse_fqdn($fqdn);
+    is_deeply(\@parsed, $expected, "FQDN is valid and compared successfully: $fqdn");
+}
+
+sub is_invalid {
+    my ($fqdn, $expected_err) = @_;
+
+    my $parsed = eval { parse_fqdn($fqdn) };
+    is($parsed, undef, "invalid FQDN did fail parsing: $fqdn");
+    is($@, $expected_err, "invalid FQDN threw correct error: $fqdn");
+}
+
+is_invalid(undef, ERR_EMPTY);
+is_invalid('', ERR_EMPTY);
+
+is_parsed('foo.example.com', ['foo', 'example.com']);
+is_parsed('foo-bar.com', ['foo-bar', 'com']);
+is_parsed('a-b.com', ['a-b', 'com']);
+
+is_invalid('foo', ERR_INVALID);
+is_invalid('-foo.com', ERR_ALPHANUM);
+is_invalid('foo-.com', ERR_ALPHANUM);
+is_invalid('foo.com-', ERR_ALPHANUM);
+is_invalid('-o-.com', ERR_ALPHANUM);
+
+# https://bugzilla.proxmox.com/show_bug.cgi?id=1054
+is_invalid('123.com', ERR_NUMERIC);
+is_parsed('foo123.com', ['foo123', 'com']);
+is_parsed('123foo.com', ['123foo', 'com']);
+
+done_testing();