Mercurial > irccd

changeset 796:1a6152af0866

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
misc: use ipv4 as option rather than family Now, all sections and JSON options that require a IP family will take ipv4 and ipv6 boolean options for convenience. It reduces parsing complexity and is more convenient for the user. Examples: # IPv6 server only [server] name = example port = 6667 hostname = example.org ipv4 = false ipv6 = true # IPv4 transport only [transport] type = ip ipv4 = true ipv6 = false port = 3320 If both options are defined (default everywhere), both protocols will be tried or bound.
author David Demelier <markand@malikania.fr>
date Sun, 11 Nov 2018 14:56:04 +0100
parents d42e8415e477
children 2dfba38e93f0
files CHANGES.md MIGRATING.md irccdctl/main.cpp libirccd/irccd/daemon/command.cpp libirccd/irccd/daemon/server_util.cpp libirccd/irccd/daemon/transport_util.cpp tests/src/libirccd/command-server-connect/main.cpp
diffstat 7 files changed, 122 insertions(+), 142 deletions(-) [+]
line wrap: on
line diff
--- a/CHANGES.md	Sun Nov 11 14:53:14 2018 +0100
+++ b/CHANGES.md	Sun Nov 11 14:56:04 2018 +0100
@@ -12,7 +12,13 @@
 - Irccd no longer supports uid, gid, pid and daemon features (#846),
 - Sections `[identity]` and `[server]` have been merged (#905),
 - Local transports support SSL (#939),
-- The origin in rule is now first class value (#947).
+- The origin in rule is now first class value (#947),
+- New option `ipv4` in `[transport]` (#945),
+- New option `ipv4` in `[server]` (#945).
+
+irccdctl:
+
+- New option `ipv4` in `[connect]` (#945).
 
 irccd-test:
 
--- a/MIGRATING.md	Sun Nov 11 14:53:14 2018 +0100
+++ b/MIGRATING.md	Sun Nov 11 14:56:04 2018 +0100
@@ -10,7 +10,6 @@
 
 - The option `reconnect-tries` has been removed from `[server]` section, use
   `auto-reconnect` boolean option instead,
-- The option `ipv6` has been removed, use `family` instead,
 - The option `reconnect-timeout` has been renamed to `auto-reconnect-delay`.
 - The section `[identity]` has been removed, instead move those values inside
   each server in their `[server]` section.
@@ -37,8 +36,6 @@
 - The request `server-mode` command requires a new argument `channel`.
 - The property `host` in request `server-connect` has been renamed to
   `hostname`,
-- The property `ipv6` in request `server-connect` has been renamed to
-  `family`,
 - The request `server-info` sends `hostname` property instead of `host`,
 - The event `onWhois` sends `hostname` property instead of `host`,
 
@@ -73,9 +70,7 @@
 - The object returned in the method `Server.info` now has a `hostname` property
   instead of `host`.
 - The property `host` in constructor `Server` has been renamed to
-  `hostname`,
-- The property `ipv6` in constructor `Server` has been renamed to
-  `family`,
+  `hostname`.
 
 #### Module ElapsedTimer
 
--- a/irccdctl/main.cpp	Sun Nov 11 14:53:14 2018 +0100
+++ b/irccdctl/main.cpp	Sun Nov 11 14:56:04 2018 +0100
@@ -79,7 +79,8 @@
  * type = "ip"
  * hostname = "ip or hostname"
  * port = "port number or service"
- * family = "ipv4, ipv6" (Optional, default: ipv4, ipv6)
+ * ipv4 = try IPv4 (Optional, default: true)
+ * ipv6 = try IPv6 (Optional, default: true)
  * ssl = true | false (Optional, default: false)
  */
 auto read_connect_ip(const ini::section& sc) -> std::unique_ptr<connector>
@@ -89,16 +90,10 @@
 	bool ipv4 = true;
 	bool ipv6 = true;
 
-	if (auto it = sc.find("family"); it != sc.end()) {
-		ipv4 = ipv6 = false;
-
-		for (auto v : *it) {
-			if (v == "ipv4")
-				ipv4 = true;
-			else if (v == "ipv6")
-				ipv6 = true;
-		}
-	}
+	if (const auto it = sc.find("ipv4"); it != sc.end())
+		ipv4 = string_util::is_boolean(it->get_value());
+	if (const auto it = sc.find("ipv6"); it != sc.end())
+		ipv6 = string_util::is_boolean(it->get_value());
 
 	if (!ipv4 && !ipv6)
 		throw transport_error(transport_error::invalid_family);
--- a/libirccd/irccd/daemon/command.cpp	Sun Nov 11 14:53:14 2018 +0100
+++ b/libirccd/irccd/daemon/command.cpp	Sun Nov 11 14:56:04 2018 +0100
@@ -533,6 +533,8 @@
 	// Optional stuff.
 	if ((server->get_options() & server::options::ipv6) == server::options::ipv6)
 		response.push_back({"ipv6", true});
+	if ((server->get_options() & server::options::ipv6) == server::options::ipv6)
+		response.push_back({"ipv6", true});
 	if ((server->get_options() & server::options::ssl) == server::options::ssl)
 		response.push_back({"ssl", true});
 	if ((server->get_options() & server::options::ssl_verify) == server::options::ssl_verify)
--- a/libirccd/irccd/daemon/server_util.cpp	Sun Nov 11 14:53:14 2018 +0100
+++ b/libirccd/irccd/daemon/server_util.cpp	Sun Nov 11 14:56:04 2018 +0100
@@ -32,6 +32,14 @@
 
 namespace {
 
+void toggle(server& s, server::options opt, bool value) noexcept
+{
+	if (value)
+		s.set_options(s.get_options() | opt);
+	else
+		s.set_options(s.get_options() & ~(opt));
+}
+
 void from_config_load_identity(server& sv, const ini::section& sc)
 {
 	const auto username = ini_util::optional_string(sc, "username", sv.get_username());
@@ -71,38 +79,32 @@
 
 void from_config_load_flags(server& sv, const ini::section& sc)
 {
-	const auto ssl = sc.get("ssl");
-	const auto ssl_verify = sc.get("ssl-verify");
-	const auto auto_rejoin = sc.get("auto-rejoin");
-	const auto auto_reconnect = sc.get("auto-reconnect");
-	const auto join_invite = sc.get("join-invite");
+	const auto ssl = sc.find("ssl");
+	const auto ssl_verify = sc.find("ssl-verify");
+	const auto auto_rejoin = sc.find("auto-rejoin");
+	const auto auto_reconnect = sc.find("auto-reconnect");
+	const auto join_invite = sc.find("join-invite");
+	const auto ipv4 = sc.find("ipv4");
+	const auto ipv6 = sc.find("ipv6");
 
-	if (const auto family = sc.find("family"); family != sc.end()) {
-		sv.set_options(sv.get_options() & ~(server::options::ipv4));
-		sv.set_options(sv.get_options() & ~(server::options::ipv6));
-
-		for (const auto& v : *family) {
-			if (v == "ipv4")
-				sv.set_options(sv.get_options() | server::options::ipv4);
-			if (v == "ipv6")
-				sv.set_options(sv.get_options() | server::options::ipv6);
-		}
-	}
+	if (ssl != sc.end())
+		toggle(sv, server::options::ssl, string_util::is_boolean(ssl->get_value()));
+	if (ssl_verify != sc.end())
+		toggle(sv, server::options::ssl_verify, string_util::is_boolean(ssl_verify->get_value()));
+	if (auto_rejoin != sc.end())
+		toggle(sv, server::options::auto_rejoin, string_util::is_boolean(auto_rejoin->get_value()));
+	if (auto_reconnect != sc.end())
+		toggle(sv, server::options::auto_reconnect, string_util::is_boolean(auto_reconnect->get_value()));
+	if (join_invite != sc.end())
+		toggle(sv, server::options::join_invite, string_util::is_boolean(join_invite->get_value()));
+	if (ipv4 != sc.end())
+		toggle(sv, server::options::ipv4, string_util::is_boolean(ipv4->get_value()));
+	if (ipv6 != sc.end())
+		toggle(sv, server::options::ipv6, string_util::is_boolean(ipv6->get_value()));
 
 	if ((sv.get_options() & server::options::ipv4) != server::options::ipv4 &&
 	    (sv.get_options() & server::options::ipv6) != server::options::ipv6)
 		throw server_error(server_error::invalid_family);
-
-	if (string_util::is_boolean(ssl.get_value()))
-		sv.set_options(sv.get_options() | server::options::ssl);
-	if (string_util::is_boolean(ssl_verify.get_value()))
-		sv.set_options(sv.get_options() | server::options::ssl_verify);
-	if (string_util::is_boolean(auto_rejoin.get_value()))
-		sv.set_options(sv.get_options() | server::options::auto_rejoin);
-	if (string_util::is_boolean(auto_reconnect.get_value()))
-		sv.set_options(sv.get_options() | server::options::auto_reconnect);
-	if (string_util::is_boolean(join_invite.get_value()))
-		sv.set_options(sv.get_options() | server::options::join_invite);
 }
 
 void from_config_load_numeric_parameters(server& sv, const ini::section& sc)
@@ -132,7 +134,7 @@
 	sv.set_command_char(command_char);
 }
 
-void from_json_load_options(server& sv, const deserializer& parser)
+void from_json_load_general(server& sv, const deserializer& parser)
 {
 	const auto port = parser.optional<std::uint16_t>("port", sv.get_port());
 	const auto nickname = parser.optional<std::string>("nickname", sv.get_nickname());
@@ -142,25 +144,6 @@
 	const auto command = parser.optional<std::string>("commandChar", sv.get_command_char());
 	const auto password = parser.optional<std::string>("password", sv.get_password());
 
-	if (const auto family = parser.find("family"); family != parser.end()) {
-		if (!family->is_array())
-			throw server_error(server_error::invalid_family);
-
-		sv.set_options(sv.get_options() & ~(server::options::ipv4));
-		sv.set_options(sv.get_options() & ~(server::options::ipv6));
-
-		for (const auto& v : *family) {
-			if (v.is_string() && v.get<std::string>() == "ipv4")
-				sv.set_options(sv.get_options() | server::options::ipv4);
-			if (v.is_string() && v.get<std::string>() == "ipv6")
-				sv.set_options(sv.get_options() | server::options::ipv6);
-		}
-	}
-
-	if ((sv.get_options() & server::options::ipv4) != server::options::ipv4 &&
-	    (sv.get_options() & server::options::ipv6) != server::options::ipv6)
-		throw server_error(server_error::invalid_family);
-
 	if (!port || *port > std::numeric_limits<std::uint16_t>::max())
 		throw server_error(server_error::invalid_port);
 	if (!nickname)
@@ -185,12 +168,20 @@
 	sv.set_password(*password);
 }
 
-void from_json_load_flags(server& sv, const deserializer& parser)
+void from_json_load_options(server& sv, const deserializer& parser)
 {
 	const auto auto_rejoin = parser.get<bool>("autoRejoin");
 	const auto join_invite = parser.get<bool>("joinInvite");
 	const auto ssl = parser.get<bool>("ssl");
 	const auto ssl_verify = parser.get<bool>("sslVerify");
+	const auto ipv4 = parser.optional<bool>("ipv4", true);
+	const auto ipv6 = parser.optional<bool>("ipv6", true);
+
+	if (!ipv4 || !ipv6)
+		throw server_error(server_error::invalid_family);
+
+	toggle(sv, server::options::ipv4, *ipv4);
+	toggle(sv, server::options::ipv6, *ipv6);
 
 	if (auto_rejoin.value_or(false))
 		sv.set_options(sv.get_options() | server::options::auto_rejoin);
@@ -205,6 +196,11 @@
 #endif
 	if (ssl_verify.value_or(false))
 		sv.set_options(sv.get_options() | server::options::ssl_verify);
+
+	// Verify that at least IPv4 or IPv6 is set.
+	if ((sv.get_options() & server::options::ipv4) != server::options::ipv4 &&
+	    (sv.get_options() & server::options::ipv6) != server::options::ipv6)
+		throw server_error(server_error::invalid_family);
 }
 
 } // !namespace
@@ -263,8 +259,8 @@
 
 	const auto sv = std::make_shared<server>(service, *id, *hostname);
 
+	from_json_load_general(*sv, parser);
 	from_json_load_options(*sv, parser);
-	from_json_load_flags(*sv, parser);
 
 	return sv;
 }
--- a/libirccd/irccd/daemon/transport_util.cpp	Sun Nov 11 14:53:14 2018 +0100
+++ b/libirccd/irccd/daemon/transport_util.cpp	Sun Nov 11 14:56:04 2018 +0100
@@ -35,27 +35,12 @@
 
 auto from_config_load_ip_protocols(const ini::section& sc) -> std::pair<bool, bool>
 {
-	bool ipv4 = true, ipv6 = false;
-
-	/*
-	 * Documentation stated family but code checked for 'domain' option.
-	 *
-	 * As irccdctl uses domain, accept both and unify the option name to 'family'.
-	 *
-	 * See #637
-	 */
-	ini::section::const_iterator it;
+	bool ipv4 = true, ipv6 = true;
 
-	if ((it = sc.find("domain")) != sc.end() || (it = sc.find("family")) != sc.end()) {
-		ipv4 = ipv6 = false;
-
-		for (const auto& v : *it) {
-			if (v == "ipv4")
-				ipv4 = true;
-			if (v == "ipv6")
-				ipv6 = true;
-		}
-	}
+	if (const auto it = sc.find("ipv4"); it != sc.end())
+		ipv4 = string_util::is_boolean(it->get_value());
+	if (const auto it = sc.find("ipv6"); it != sc.end())
+		ipv6 = string_util::is_boolean(it->get_value());
 
 	if (!ipv4 && !ipv6)
 		throw transport_error(transport_error::invalid_family);
--- a/tests/src/libirccd/command-server-connect/main.cpp	Sun Nov 11 14:53:14 2018 +0100
+++ b/tests/src/libirccd/command-server-connect/main.cpp	Sun Nov 11 14:56:04 2018 +0100
@@ -51,21 +51,22 @@
 BOOST_AUTO_TEST_CASE(full)
 {
 	const auto [json, code] = request({
-		{ "command",    "server-connect"    },
-		{ "name",       "local2"            },
-		{ "hostname",   "irc.example2.org"  },
-		{ "password",   "nonono"            },
-		{ "nickname",   "francis"           },
-		{ "realname",   "the_francis"       },
-		{ "username",   "frc"               },
-		{ "family",     { "ipv6" }          },
-		{ "ctcpVersion", "ultra bot"        },
-		{ "commandChar", "::"               },
-		{ "port",       18000               },
-		{ "ssl",        true                },
-		{ "sslVerify",  true                },
-		{ "autoRejoin", true                },
-		{ "joinInvite", true                }
+		{ "command",            "server-connect"        },
+		{ "name",               "local2"                },
+		{ "hostname",           "irc.example2.org"      },
+		{ "password",           "nonono"                },
+		{ "nickname",           "francis"               },
+		{ "realname",           "the_francis"           },
+		{ "username",           "frc"                   },
+		{ "ipv4",               false                   },
+		{ "ipv6",               true                    },
+		{ "ctcpVersion",        "ultra bot"             },
+		{ "commandChar",        "::"                    },
+		{ "port",               18000                   },
+		{ "ssl",                true                    },
+		{ "sslVerify",          true                    },
+		{ "autoRejoin",         true                    },
+		{ "joinInvite",         true                    }
 	});
 
 	const auto s = irccd_.servers().get("local2");
@@ -98,9 +99,9 @@
 	irccd_.servers().add(std::make_unique<mock_server>(ctx_, "local"));
 
 	const auto [json, code] = request({
-		{ "command",    "server-connect"    },
-		{ "name",       "local"             },
-		{ "hostname",   "127.0.0.1"         }
+		{ "command",    "server-connect"        },
+		{ "name",       "local"                 },
+		{ "hostname",   "127.0.0.1"             }
 	});
 
 	BOOST_TEST(code == server_error::already_exists);
@@ -111,8 +112,8 @@
 BOOST_AUTO_TEST_CASE(invalid_hostname_1)
 {
 	const auto [json, code] = request({
-		{ "command",    "server-connect"    },
-		{ "name",       "new"               },
+		{ "command",    "server-connect"        },
+		{ "name",       "new"                   },
 	});
 
 	BOOST_TEST(code == server_error::invalid_hostname);
@@ -123,9 +124,9 @@
 BOOST_AUTO_TEST_CASE(invalid_hostname_2)
 {
 	const auto [json, code] = request({
-		{ "command",    "server-connect"    },
-		{ "name",       "new"               },
-		{ "hostname",   123456              }
+		{ "command",    "server-connect"        },
+		{ "name",       "new"                   },
+		{ "hostname",   123456                  }
 	});
 
 	BOOST_TEST(code == server_error::invalid_hostname);
@@ -136,9 +137,9 @@
 BOOST_AUTO_TEST_CASE(invalid_identifier_1)
 {
 	const auto [json, code] = request({
-		{ "command",    "server-connect"    },
-		{ "name",       ""                  },
-		{ "hostname",   "127.0.0.1"         }
+		{ "command",    "server-connect"        },
+		{ "name",       ""                      },
+		{ "hostname",   "127.0.0.1"             }
 	});
 
 	BOOST_TEST(code == server_error::invalid_identifier);
@@ -149,9 +150,9 @@
 BOOST_AUTO_TEST_CASE(invalid_identifier_2)
 {
 	const auto [json, code] = request({
-		{ "command",    "server-connect"    },
-		{ "name",       123456              },
-		{ "hostname",   "127.0.0.1"         }
+		{ "command",    "server-connect"        },
+		{ "name",       123456                  },
+		{ "hostname",   "127.0.0.1"             }
 	});
 
 	BOOST_TEST(code == server_error::invalid_identifier);
@@ -162,10 +163,10 @@
 BOOST_AUTO_TEST_CASE(invalid_port_1)
 {
 	const auto [json, code] = request({
-		{ "command",    "server-connect"    },
-		{ "name",       "new"               },
-		{ "hostname",   "127.0.0.1"         },
-		{ "port",       "notaint"           }
+		{ "command",    "server-connect"        },
+		{ "name",       "new"                   },
+		{ "hostname",   "127.0.0.1"             },
+		{ "port",       "notaint"               }
 	});
 
 	BOOST_TEST(code == server_error::invalid_port);
@@ -176,10 +177,10 @@
 BOOST_AUTO_TEST_CASE(invalid_port_2)
 {
 	const auto [json, code] = request({
-		{ "command",    "server-connect"    },
-		{ "name",       "new"               },
-		{ "hostname",   "127.0.0.1"         },
-		{ "port",       -123                }
+		{ "command",    "server-connect"        },
+		{ "name",       "new"                   },
+		{ "hostname",   "127.0.0.1"             },
+		{ "port",       -123                    }
 	});
 
 	BOOST_TEST(code == server_error::invalid_port);
@@ -190,10 +191,10 @@
 BOOST_AUTO_TEST_CASE(invalid_port_3)
 {
 	const auto [json, code] = request({
-		{ "command",    "server-connect"    },
-		{ "name",       "new"               },
-		{ "hostname",   "127.0.0.1"         },
-		{ "port",       1000000             }
+		{ "command",    "server-connect"        },
+		{ "name",       "new"                   },
+		{ "hostname",   "127.0.0.1"             },
+		{ "port",       1000000                 }
 	});
 
 	BOOST_TEST(code == server_error::invalid_port);
@@ -206,10 +207,10 @@
 BOOST_AUTO_TEST_CASE(ssl_disabled)
 {
 	const auto [json, code] = request({
-		{ "command",    "server-connect"    },
-		{ "name",       "new"               },
-		{ "hostname",   "127.0.0.1"         },
-		{ "ssl",        true                }
+		{ "command",    "server-connect"        },
+		{ "name",       "new"                   },
+		{ "hostname",   "127.0.0.1"             },
+		{ "ssl",        true                    }
 	});
 
 	BOOST_TEST(code == server_error::ssl_disabled);
@@ -222,11 +223,11 @@
 BOOST_AUTO_TEST_CASE(invalid_family_1)
 {
 	const auto [json, code] = request({
-		{ "command",    "server-connect"    },
-		{ "name",       "new"               },
-		{ "hostname",   "127.0.0.1"         },
-		{ "port",       1000000             },
-		{ "family",     "invalid"           }
+		{ "command",    "server-connect"        },
+		{ "name",       "new"                   },
+		{ "hostname",   "127.0.0.1"             },
+		{ "port",       6667                    },
+		{ "ipv4",       "invalid"               }
 	});
 
 	BOOST_TEST(code == server_error::invalid_family);
@@ -237,11 +238,11 @@
 BOOST_AUTO_TEST_CASE(invalid_family_2)
 {
 	const auto [json, code] = request({
-		{ "command",    "server-connect"    },
-		{ "name",       "new"               },
-		{ "hostname",   "127.0.0.1"         },
-		{ "port",       1000000             },
-		{ "family",     { 123, 456 }        }
+		{ "command",    "server-connect"        },
+		{ "name",       "new"                   },
+		{ "hostname",   "127.0.0.1"             },
+		{ "port",       6667                    },
+		{ "ipv6",       1234                    }
 	});
 
 	BOOST_TEST(code == server_error::invalid_family);