From 8fb494435889dcb9e1c09b8c7220e47bab717bf9 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 27 Jul 2015 22:15:26 -0400 Subject: [PATCH] hostname-util: add relax parameter to hostname_is_valid Tests are modified to check behaviour with relax and without relax. New tests are added for hostname_cleanup(). Tests are moved a new file (test-hostname-util) because there's now a bunch of them. New parameter is not used anywhere, except in tests, so there should be no observable change. --- .gitignore | 1 + Makefile.am | 7 ++ src/basic/hostname-util.c | 15 ++- src/basic/hostname-util.h | 2 +- src/basic/util.c | 2 +- src/basic/util.h | 5 + src/firstboot/firstboot.c | 8 +- src/hostname/hostnamed.c | 4 +- src/import/pull-dkr.c | 2 +- src/libsystemd-network/sd-dhcp-lease.c | 5 +- src/locale/localed.c | 5 - src/network/networkd-network.c | 2 +- src/test/test-hostname-util.c | 163 +++++++++++++++++++++++++ src/test/test-util.c | 69 ----------- 14 files changed, 199 insertions(+), 91 deletions(-) create mode 100644 src/test/test-hostname-util.c diff --git a/.gitignore b/.gitignore index 7659e7a645b..a66c7e17759 100644 --- a/.gitignore +++ b/.gitignore @@ -197,6 +197,7 @@ /test-firewall-util /test-hashmap /test-hostname +/test-hostname-util /test-icmp6-rs /test-id128 /test-inhibit diff --git a/Makefile.am b/Makefile.am index d21982285f2..4f97da10c29 100644 --- a/Makefile.am +++ b/Makefile.am @@ -1393,6 +1393,7 @@ tests += \ test-utf8 \ test-ellipsize \ test-util \ + test-hostname-util \ test-process-util \ test-terminal-util \ test-path-lookup \ @@ -1671,6 +1672,12 @@ test_util_SOURCES = \ test_util_LDADD = \ libshared.la +test_hostname_util_SOURCES = \ + src/test/test-hostname-util.c + +test_hostname_util_LDADD = \ + libshared.la + test_process_util_SOURCES = \ src/test/test-process-util.c diff --git a/src/basic/hostname-util.c b/src/basic/hostname-util.c index e336f269fae..21fae89fd00 100644 --- a/src/basic/hostname-util.c +++ b/src/basic/hostname-util.c @@ -61,14 +61,22 @@ static bool hostname_valid_char(char c) { c == '.'; } -bool hostname_is_valid(const char *s) { +/** + * Check if s looks like a valid host name or fqdn. This does not do + * full DNS validation, but only checks if the name is composed of + * allowed characters and the length is not above the maximum allowed + * by Linux (c.f. dns_name_is_valid()). Trailing dot is allowed if + * relax is true and at least two components are present in the name. + */ +bool hostname_is_valid(const char *s, bool relax) { const char *p; bool dot; + unsigned dots = 0; if (isempty(s)) return false; - /* Doesn't accept empty hostnames, hostnames with trailing or + /* Doesn't accept empty hostnames, hostnames with * leading dots, and hostnames with multiple dots in a * sequence. Also ensures that the length stays below * HOST_NAME_MAX. */ @@ -79,6 +87,7 @@ bool hostname_is_valid(const char *s) { return false; dot = true; + dots ++; } else { if (!hostname_valid_char(*p)) return false; @@ -87,7 +96,7 @@ bool hostname_is_valid(const char *s) { } } - if (dot) + if (dot && (dots < 2 || !relax)) return false; if (p-s > HOST_NAME_MAX) diff --git a/src/basic/hostname-util.h b/src/basic/hostname-util.h index 0c4763cf5a3..ecae3a267ce 100644 --- a/src/basic/hostname-util.h +++ b/src/basic/hostname-util.h @@ -29,7 +29,7 @@ bool hostname_is_set(void); char* gethostname_malloc(void); -bool hostname_is_valid(const char *s) _pure_; +bool hostname_is_valid(const char *s, bool relax) _pure_; char* hostname_cleanup(char *s, bool lowercase); bool is_localhost(const char *hostname); diff --git a/src/basic/util.c b/src/basic/util.c index 7896be87885..f81bf4e31e6 100644 --- a/src/basic/util.c +++ b/src/basic/util.c @@ -2997,7 +2997,7 @@ char* strshorten(char *s, size_t l) { bool machine_name_is_valid(const char *s) { - if (!hostname_is_valid(s)) + if (!hostname_is_valid(s, false)) return false; /* Machine names should be useful hostnames, but also be diff --git a/src/basic/util.h b/src/basic/util.h index c2e5cc610b9..5c0130d265e 100644 --- a/src/basic/util.h +++ b/src/basic/util.h @@ -858,6 +858,11 @@ int unquote_first_word(const char **p, char **ret, UnquoteFlags flags); int unquote_first_word_and_warn(const char **p, char **ret, UnquoteFlags flags, const char *unit, const char *filename, unsigned line, const char *rvalue); int unquote_many_words(const char **p, UnquoteFlags flags, ...) _sentinel_; +static inline void free_and_replace(char **s, char *v) { + free(*s); + *s = v; +} + int free_and_strdup(char **p, const char *s); #define INOTIFY_EVENT_MAX (sizeof(struct inotify_event) + NAME_MAX + 1) diff --git a/src/firstboot/firstboot.c b/src/firstboot/firstboot.c index 3805b294373..43669d28bf0 100644 --- a/src/firstboot/firstboot.c +++ b/src/firstboot/firstboot.c @@ -386,7 +386,7 @@ static int prompt_hostname(void) { break; } - if (!hostname_is_valid(h)) { + if (!hostname_is_valid(h, false)) { log_error("Specified hostname invalid."); continue; } @@ -780,14 +780,12 @@ static int parse_argv(int argc, char *argv[]) { break; case ARG_HOSTNAME: - if (!hostname_is_valid(optarg)) { + if (!hostname_is_valid(optarg, false)) { log_error("Host name %s is not valid.", optarg); return -EINVAL; } - free(arg_hostname); - arg_hostname = strdup(optarg); - if (!arg_hostname) + if (free_and_strdup(&arg_hostname, optarg) < 0) return log_oom(); break; diff --git a/src/hostname/hostnamed.c b/src/hostname/hostnamed.c index e52b872a8c7..a78516c8b53 100644 --- a/src/hostname/hostnamed.c +++ b/src/hostname/hostnamed.c @@ -424,7 +424,7 @@ static int method_set_hostname(sd_bus_message *m, void *userdata, sd_bus_error * if (isempty(name)) name = "localhost"; - if (!hostname_is_valid(name)) + if (!hostname_is_valid(name, false)) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid hostname '%s'", name); if (streq_ptr(name, c->data[PROP_HOSTNAME])) @@ -501,7 +501,7 @@ static int method_set_static_hostname(sd_bus_message *m, void *userdata, sd_bus_ } else { char *h; - if (!hostname_is_valid(name)) + if (!hostname_is_valid(name, false)) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid static hostname '%s'", name); h = strdup(name); diff --git a/src/import/pull-dkr.c b/src/import/pull-dkr.c index 67ca1ce8e40..53d5174f332 100644 --- a/src/import/pull-dkr.c +++ b/src/import/pull-dkr.c @@ -721,7 +721,7 @@ static int dkr_pull_job_on_header(PullJob *j, const char *header, size_t sz) { return log_oom(); STRV_FOREACH(k, l) { - if (!hostname_is_valid(*k)) { + if (!hostname_is_valid(*k, false)) { log_error("Registry hostname is not valid."); strv_free(l); return -EBADMSG; diff --git a/src/libsystemd-network/sd-dhcp-lease.c b/src/libsystemd-network/sd-dhcp-lease.c index 54417b3af38..43f7566f651 100644 --- a/src/libsystemd-network/sd-dhcp-lease.c +++ b/src/libsystemd-network/sd-dhcp-lease.c @@ -553,11 +553,10 @@ int dhcp_lease_parse_options(uint8_t code, uint8_t len, const uint8_t *option, if (e) *e = 0; - if (!hostname_is_valid(hostname) || is_localhost(hostname)) + if (!hostname_is_valid(hostname, false) || is_localhost(hostname)) break; - free(lease->hostname); - lease->hostname = hostname; + free_and_replace(&lease->hostname, hostname); hostname = NULL; break; diff --git a/src/locale/localed.c b/src/locale/localed.c index 88756542fdb..3b511bfaf4a 100644 --- a/src/locale/localed.c +++ b/src/locale/localed.c @@ -99,11 +99,6 @@ static const char* nonempty(const char *s) { return isempty(s) ? NULL : s; } -static void free_and_replace(char **s, char *v) { - free(*s); - *s = v; -} - static bool startswith_comma(const char *s, const char *prefix) { const char *t; diff --git a/src/network/networkd-network.c b/src/network/networkd-network.c index d8f42621af2..e3ac70ff333 100644 --- a/src/network/networkd-network.c +++ b/src/network/networkd-network.c @@ -838,7 +838,7 @@ int config_parse_hostname(const char *unit, if (r < 0) return r; - if (!hostname_is_valid(hn)) { + if (!hostname_is_valid(hn, false)) { log_syntax(unit, LOG_ERR, filename, line, EINVAL, "hostname is not valid, ignoring assignment: %s", rvalue); free(hn); diff --git a/src/test/test-hostname-util.c b/src/test/test-hostname-util.c new file mode 100644 index 00000000000..2357ab5758b --- /dev/null +++ b/src/test/test-hostname-util.c @@ -0,0 +1,163 @@ +/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/ + +/*** + This file is part of systemd. + + Copyright 2010 Lennart Poettering + Copyright 2013 Thomas H.P. Andersen + Copyright 2015 Zbigniew Jędrzejewski-Szmek + + systemd is free software; you can redistribute it and/or modify it + under the terms of the GNU Lesser General Public License as published by + the Free Software Foundation; either version 2.1 of the License, or + (at your option) any later version. + + systemd is distributed in the hope that it will be useful, but + WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public License + along with systemd; If not, see . +***/ + +#include "util.h" +#include "fileio.h" +#include "hostname-util.h" + +static void test_hostname_is_valid(void) { + assert_se(hostname_is_valid("foobar", false)); + assert_se(hostname_is_valid("foobar.com", false)); + assert_se(!hostname_is_valid("foobar.com.", false)); + assert_se(hostname_is_valid("fooBAR", false)); + assert_se(hostname_is_valid("fooBAR.com", false)); + assert_se(!hostname_is_valid("fooBAR.", false)); + assert_se(!hostname_is_valid("fooBAR.com.", false)); + assert_se(!hostname_is_valid("fööbar", false)); + assert_se(!hostname_is_valid("", false)); + assert_se(!hostname_is_valid(".", false)); + assert_se(!hostname_is_valid("..", false)); + assert_se(!hostname_is_valid("foobar.", false)); + assert_se(!hostname_is_valid(".foobar", false)); + assert_se(!hostname_is_valid("foo..bar", false)); + assert_se(!hostname_is_valid("foo.bar..", false)); + assert_se(!hostname_is_valid("xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx", false)); + + assert_se(hostname_is_valid("foobar", true)); + assert_se(hostname_is_valid("foobar.com", true)); + assert_se(hostname_is_valid("foobar.com.", true)); + assert_se(hostname_is_valid("fooBAR", true)); + assert_se(hostname_is_valid("fooBAR.com", true)); + assert_se(!hostname_is_valid("fooBAR.", true)); + assert_se(hostname_is_valid("fooBAR.com.", true)); + assert_se(!hostname_is_valid("fööbar", true)); + assert_se(!hostname_is_valid("", true)); + assert_se(!hostname_is_valid(".", true)); + assert_se(!hostname_is_valid("..", true)); + assert_se(!hostname_is_valid("foobar.", true)); + assert_se(!hostname_is_valid(".foobar", true)); + assert_se(!hostname_is_valid("foo..bar", true)); + assert_se(!hostname_is_valid("foo.bar..", true)); + assert_se(!hostname_is_valid("xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx", true)); +} + +static void test_hostname_cleanup(void) { + char *s; + + s = strdupa("foobar"); + assert_se(streq(hostname_cleanup(s, false), "foobar")); + s = strdupa("foobar.com"); + assert_se(streq(hostname_cleanup(s, false), "foobar.com")); + s = strdupa("foobar.com."); + assert_se(streq(hostname_cleanup(s, false), "foobar.com")); + s = strdupa("fooBAR"); + assert_se(streq(hostname_cleanup(s, false), "fooBAR")); + s = strdupa("fooBAR.com"); + assert_se(streq(hostname_cleanup(s, false), "fooBAR.com")); + s = strdupa("fooBAR."); + assert_se(streq(hostname_cleanup(s, false), "fooBAR")); + s = strdupa("fooBAR.com."); + assert_se(streq(hostname_cleanup(s, false), "fooBAR.com")); + s = strdupa("fööbar"); + assert_se(streq(hostname_cleanup(s, false), "fbar")); + s = strdupa(""); + assert_se(isempty(hostname_cleanup(s, false))); + s = strdupa("."); + assert_se(isempty(hostname_cleanup(s, false))); + s = strdupa(".."); + assert_se(isempty(hostname_cleanup(s, false))); + s = strdupa("foobar."); + assert_se(streq(hostname_cleanup(s, false), "foobar")); + s = strdupa(".foobar"); + assert_se(streq(hostname_cleanup(s, false), "foobar")); + s = strdupa("foo..bar"); + assert_se(streq(hostname_cleanup(s, false), "foo.bar")); + s = strdupa("foo.bar.."); + assert_se(streq(hostname_cleanup(s, false), "foo.bar")); + s = strdupa("xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"); + assert_se(streq(hostname_cleanup(s, false), "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx")); +} + +static void test_read_hostname_config(void) { + char path[] = "/tmp/hostname.XXXXXX"; + char *hostname; + int fd; + + fd = mkostemp_safe(path, O_RDWR|O_CLOEXEC); + assert(fd > 0); + close(fd); + + /* simple hostname */ + write_string_file(path, "foo", WRITE_STRING_FILE_CREATE); + assert_se(read_hostname_config(path, &hostname) == 0); + assert_se(streq(hostname, "foo")); + free(hostname); + hostname = NULL; + + /* with comment */ + write_string_file(path, "# comment\nfoo", WRITE_STRING_FILE_CREATE); + assert_se(read_hostname_config(path, &hostname) == 0); + assert_se(hostname); + assert_se(streq(hostname, "foo")); + free(hostname); + hostname = NULL; + + /* with comment and extra whitespace */ + write_string_file(path, "# comment\n\n foo ", WRITE_STRING_FILE_CREATE); + assert_se(read_hostname_config(path, &hostname) == 0); + assert_se(hostname); + assert_se(streq(hostname, "foo")); + free(hostname); + hostname = NULL; + + /* cleans up name */ + write_string_file(path, "!foo/bar.com", WRITE_STRING_FILE_CREATE); + assert_se(read_hostname_config(path, &hostname) == 0); + assert_se(hostname); + assert_se(streq(hostname, "foobar.com")); + free(hostname); + hostname = NULL; + + /* no value set */ + hostname = (char*) 0x1234; + write_string_file(path, "# nothing here\n", WRITE_STRING_FILE_CREATE); + assert_se(read_hostname_config(path, &hostname) == -ENOENT); + assert_se(hostname == (char*) 0x1234); /* does not touch argument on error */ + + /* nonexisting file */ + assert_se(read_hostname_config("/non/existing", &hostname) == -ENOENT); + assert_se(hostname == (char*) 0x1234); /* does not touch argument on error */ + + unlink(path); +} + +int main(int argc, char *argv[]) { + log_parse_environment(); + log_open(); + + test_hostname_is_valid(); + test_hostname_cleanup(); + test_read_hostname_config(); + + return 0; +} diff --git a/src/test/test-util.c b/src/test/test-util.c index f43433baa18..9ad912d20f8 100644 --- a/src/test/test-util.c +++ b/src/test/test-util.c @@ -38,7 +38,6 @@ #include "conf-parser.h" #include "virt.h" #include "process-util.h" -#include "hostname-util.h" #include "signal-util.h" static void test_streq_ptr(void) { @@ -833,72 +832,6 @@ static void test_memdup_multiply(void) { free(dup); } -static void test_hostname_is_valid(void) { - assert_se(hostname_is_valid("foobar")); - assert_se(hostname_is_valid("foobar.com")); - assert_se(!hostname_is_valid("fööbar")); - assert_se(!hostname_is_valid("")); - assert_se(!hostname_is_valid(".")); - assert_se(!hostname_is_valid("..")); - assert_se(!hostname_is_valid("foobar.")); - assert_se(!hostname_is_valid(".foobar")); - assert_se(!hostname_is_valid("foo..bar")); - assert_se(!hostname_is_valid("xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx")); -} - -static void test_read_hostname_config(void) { - char path[] = "/tmp/hostname.XXXXXX"; - char *hostname; - int fd; - - fd = mkostemp_safe(path, O_RDWR|O_CLOEXEC); - assert(fd > 0); - close(fd); - - /* simple hostname */ - write_string_file(path, "foo", WRITE_STRING_FILE_CREATE); - assert_se(read_hostname_config(path, &hostname) == 0); - assert_se(streq(hostname, "foo")); - free(hostname); - hostname = NULL; - - /* with comment */ - write_string_file(path, "# comment\nfoo", WRITE_STRING_FILE_CREATE); - assert_se(read_hostname_config(path, &hostname) == 0); - assert_se(hostname); - assert_se(streq(hostname, "foo")); - free(hostname); - hostname = NULL; - - /* with comment and extra whitespace */ - write_string_file(path, "# comment\n\n foo ", WRITE_STRING_FILE_CREATE); - assert_se(read_hostname_config(path, &hostname) == 0); - assert_se(hostname); - assert_se(streq(hostname, "foo")); - free(hostname); - hostname = NULL; - - /* cleans up name */ - write_string_file(path, "!foo/bar.com", WRITE_STRING_FILE_CREATE); - assert_se(read_hostname_config(path, &hostname) == 0); - assert_se(hostname); - assert_se(streq(hostname, "foobar.com")); - free(hostname); - hostname = NULL; - - /* no value set */ - hostname = (char*) 0x1234; - write_string_file(path, "# nothing here\n", WRITE_STRING_FILE_CREATE); - assert_se(read_hostname_config(path, &hostname) == -ENOENT); - assert_se(hostname == (char*) 0x1234); /* does not touch argument on error */ - - /* nonexisting file */ - assert_se(read_hostname_config("/non/existing", &hostname) == -ENOENT); - assert_se(hostname == (char*) 0x1234); /* does not touch argument on error */ - - unlink(path); -} - static void test_u64log2(void) { assert_se(u64log2(0) == 0); assert_se(u64log2(8) == 3); @@ -2124,8 +2057,6 @@ int main(int argc, char *argv[]) { test_foreach_word(); test_foreach_word_quoted(); test_memdup_multiply(); - test_hostname_is_valid(); - test_read_hostname_config(); test_u64log2(); test_protect_errno(); test_parse_size(); -- 2.39.2