Skip to content

Disallow map section URI paths which are parents of subsequent map section URI paths #486

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
hummeltech wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Disallow map section URI paths which are parents of subsequent map section URI paths #486

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion etc/apache2/renderd-example-map.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,5 +161,5 @@ Listen 8081
# Off = a 404 is returned for that url instead.
# Default: On
#ModTileEnableDirtyURL Off

LogLevel debug
</VirtualHost>
42 changes: 38 additions & 4 deletions src/renderd_config.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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)
Expand Down
47 changes: 47 additions & 0 deletions tests/renderd_config_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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'"));
}
}

Loading