Disallow map section URIs which are subpaths of subsequent map section URIs

Author: hummeltech

etc/apache2/renderd-example-map.conf

@@ -161,5 +161,5 @@ Listen 8081
     # Off = a 404 is returned for that url instead.
     # Default: On
     #ModTileEnableDirtyURL Off
-
+    LogLevel debug
 </VirtualHost>

src/renderd_config.c

@@ -106,6 +106,31 @@ static void process_config_string(const dictionary *ini, const char *section, co
 	free(key);
 }
 
+static char *process_config_string_with_trailing_slash(const dictionary *ini, const char *section, const char *name, const char **dest, const char *notfound, size_t maxlen)
+{
+	char *key = name_with_section(section, name);
+	const char *src = iniparser_getstring(ini, key, notfound);
+
+	g_logger(G_LOG_LEVEL_DEBUG, "\tRead %s: '%s'", key, src);
+
+	if (src[strnlen(src, maxlen) - 1] != '/') {
+		char *tempsrc = strndup(src, maxlen);
+		int len = asprintf(&tempsrc, "%s/", src);
+
+		if (len == -1) {
+			g_logger(G_LOG_LEVEL_CRITICAL, "process_config_string_with_trailing_slash: asprintf error");
+			exit(7);
+		}
+
+		copy_string(tempsrc, dest, maxlen);
+		free(tempsrc);
+	} else {
+		copy_string(src, dest, maxlen);
+	}
+
+	free(key);
+}
+
 void free_map_section(xmlconfigitem map_section)
 {
 	free((void *)map_section.attribution);
@@ -239,8 +264,8 @@ void process_map_sections(const char *config_file_name, xmlconfigitem *maps_dest
 			process_config_string(ini, section, "parameterize_style", &maps_dest[map_section_num].parameterization, "", PATH_MAX);
 			process_config_string(ini, section, "server_alias", &maps_dest[map_section_num].server_alias, "", PATH_MAX);
 			process_config_string(ini, section, "tiledir", &maps_dest[map_section_num].tile_dir, default_tile_dir, PATH_MAX);
-			process_config_string(ini, section, "uri", &maps_dest[map_section_num].xmluri, "", PATH_MAX);
 			process_config_string(ini, section, "xml", &maps_dest[map_section_num].xmlfile, "", PATH_MAX);
+			process_config_string_with_trailing_slash(ini, section, "uri", &maps_dest[map_section_num].xmluri, "", PATH_MAX);
 
 			process_config_double(ini, section, "scale", &maps_dest[map_section_num].scale_factor, 1.0);
 
@@ -275,9 +300,7 @@ void process_map_sections(const char *config_file_name, xmlconfigitem *maps_dest
 			process_config_string(ini, section, "type", &ini_type, "png image/png png256", INILINE_MAX);
 			ini_type_copy = strndup(ini_type, INILINE_MAX);
 
-			for (ini_type_part = strtok_r(ini_type_copy, " ", &ini_type_context);
-					ini_type_part;
-					ini_type_part = strtok_r(NULL, " ", &ini_type_context)) {
+			for (ini_type_part = strtok_r(ini_type_copy, " ", &ini_type_context); ini_type_part; ini_type_part = strtok_r(NULL, " ", &ini_type_context)) {
 				switch (ini_type_part_num) {
 					case 0:
 						copy_string(ini_type_part, &maps_dest[map_section_num].file_extension, ini_type_part_maxlen);
@@ -329,6 +352,17 @@ void process_map_sections(const char *config_file_name, xmlconfigitem *maps_dest
 		g_logger(G_LOG_LEVEL_CRITICAL, "No map config sections were found in file: %s", config_file_name);
 		exit(1);
 	}
+
+	if (map_section_num > 0) {
+		for (int x = 0; x < map_section_num; x++) {
+			for (int y = x + 1; y <= map_section_num; y++) {
+				if (strncmp(maps_dest[x].xmluri, maps_dest[y].xmluri, strnlen(maps_dest[x].xmluri, PATH_MAX)) == 0) {
+					g_logger(G_LOG_LEVEL_CRITICAL, "Specified URI path ('%s' in map section '%s') must not be the parent of any subsequent map config section's URI path, e.g., '%s' in map section '%s'.", maps_dest[x].xmluri, maps_dest[x].xmlname, maps_dest[y].xmluri, maps_dest[y].xmlname);
+					exit(7);
+				}
+			}
+		}
+	}
 }
 
 void process_mapnik_section(const char *config_file_name, renderd_config *config_dest)

tests/renderd_config_test.cpp

@@ -187,6 +187,7 @@ TEST_CASE("renderd_config config parser", "specific testing")
 
 		for (int i = 0; i <= XMLCONFIGS_MAX; i++) {
 			renderd_conf_file << "[map" + std::to_string(i) + "]\n";
+			renderd_conf_file << "uri=/" + std::to_string(i) + "\n";
 		}
 
 		renderd_conf_file.close();
@@ -447,4 +448,50 @@ TEST_CASE("renderd_config config parser", "specific testing")
 		REQUIRE(WEXITSTATUS(status) == 7);
 		REQUIRE_THAT(err_log_lines, Catch::Matchers::Contains("Duplicate renderd config section names for section 0: renderd0 & renderd"));
 	}
+
+	SECTION("renderd.conf with overlapping URIs", "should return 7") {
+		std::string map0_uri = GENERATE("/", "/map1/", "/map2/");
+
+		std::string renderd_conf = std::tmpnam(nullptr);
+		std::ofstream renderd_conf_file;
+		renderd_conf_file.open(renderd_conf);
+		renderd_conf_file << "[mapnik]\n[renderd]\n";
+
+		renderd_conf_file << "[map0]\n";
+		renderd_conf_file << "uri=" + map0_uri + "\n";
+		renderd_conf_file << "[map1]\n";
+		renderd_conf_file << "uri=" + map0_uri + "1\n";
+
+		renderd_conf_file.close();
+
+		std::vector<std::string> argv = {"--config", renderd_conf};
+
+		int status = run_command(test_binary, argv);
+		std::remove(renderd_conf.c_str());
+		REQUIRE(WEXITSTATUS(status) == 7);
+		REQUIRE_THAT(err_log_lines, Catch::Matchers::Contains("Specified URI path ('" + map0_uri + "' in map section 'map0') must not be the parent of any subsequent map config section's URI path, e.g., '" + map0_uri + "1/' in map section 'map1'"));
+	}
+
+	SECTION("renderd.conf with blank URIs", "should return 7") {
+		std::string renderd_conf = std::tmpnam(nullptr);
+		std::ofstream renderd_conf_file;
+		renderd_conf_file.open(renderd_conf);
+		renderd_conf_file << "[mapnik]\n[renderd]\n";
+
+		renderd_conf_file << "[map0]\n";
+		renderd_conf_file << "uri=/0\n";
+		renderd_conf_file << "[map1]\n";
+		renderd_conf_file << "[map2]\n";
+		renderd_conf_file << "uri=/2\n";
+
+		renderd_conf_file.close();
+
+		std::vector<std::string> argv = {"--config", renderd_conf};
+
+		int status = run_command(test_binary, argv);
+		std::remove(renderd_conf.c_str());
+		REQUIRE(WEXITSTATUS(status) == 7);
+		REQUIRE_THAT(err_log_lines, Catch::Matchers::Contains("Specified URI path ('/' in map section 'map1') must not be the parent of any subsequent map config section's URI path, e.g., '/2/' in map section 'map2'"));
+	}
 }
+