The CGit configuration variable virtual_root is normalized so that it does not have a trailing '/' character, but it is allowed to be empty (the empty string and NULL have different meanings here) and there is code that is insufficiently cautious when checking if it ends in a '/':
if (virtual_root[strlen(virtual_root) - 1] != '/') Clearly this check is redundant, but rather than simply removing it we get a slight efficiency improvement by switching the normalization so that the virtual_root variable always ends in '/'. Do this with a new "ensure_end" helper. Signed-off-by: John Keeping <j...@keeping.me.uk> --- I tried running the CGit test suite with cgit compiled using Clang's -fsanitize=address and -fsanitize=undefined options. This is the only issue I found. cgit.c | 11 +++-------- cgit.h | 3 ++- shared.c | 15 +++++++++++++++ ui-shared.c | 14 +++++--------- 4 files changed, 25 insertions(+), 18 deletions(-) diff --git a/cgit.c b/cgit.c index afafcce..ef24fc3 100644 --- a/cgit.c +++ b/cgit.c @@ -155,9 +155,7 @@ static void config_cb(const char *name, const char *value) else if (!strcmp(name, "strict-export")) ctx.cfg.strict_export = xstrdup(value); else if (!strcmp(name, "virtual-root")) { - ctx.cfg.virtual_root = trim_end(value, '/'); - if (!ctx.cfg.virtual_root && (!strcmp(value, "/"))) - ctx.cfg.virtual_root = ""; + ctx.cfg.virtual_root = ensure_end(value, '/'); } else if (!strcmp(name, "nocache")) ctx.cfg.nocache = atoi(value); else if (!strcmp(name, "noplainemail")) @@ -838,11 +836,8 @@ int main(int argc, const char **argv) * that virtual-root equals SCRIPT_NAME, minus any possibly * trailing slashes. */ - if (!ctx.cfg.virtual_root && ctx.cfg.script_name) { - ctx.cfg.virtual_root = trim_end(ctx.cfg.script_name, '/'); - if (!ctx.cfg.virtual_root) - ctx.cfg.virtual_root = ""; - } + if (!ctx.cfg.virtual_root && ctx.cfg.script_name) + ctx.cfg.virtual_root = ensure_end(ctx.cfg.script_name, '/'); /* If no url parameter is specified on the querystring, lets * use PATH_INFO as url. This allows cgit to work with virtual diff --git a/cgit.h b/cgit.h index ed5cf14..5033cc2 100644 --- a/cgit.h +++ b/cgit.h @@ -190,7 +190,7 @@ struct cgit_config { char *script_name; char *section; char *repository_sort; - char *virtual_root; + char *virtual_root; /* Always ends with '/'. */ char *strict_export; int cache_size; int cache_dynamic_ttl; @@ -300,6 +300,7 @@ extern int chk_positive(int result, char *msg); extern int chk_non_negative(int result, char *msg); extern char *trim_end(const char *str, char c); +extern char *ensure_end(const char *str, char c); extern char *strlpart(char *txt, int maxlen); extern char *strrpart(char *txt, int maxlen); diff --git a/shared.c b/shared.c index cc06930..1fa9c99 100644 --- a/shared.c +++ b/shared.c @@ -115,6 +115,21 @@ char *trim_end(const char *str, char c) return xstrndup(str, len); } +char *ensure_end(const char *str, char c) +{ + size_t len = strlen(str); + char *result; + + if (len && str[len - 1] == c) + return xstrndup(str, len); + + result = xmalloc(len + 2); + memcpy(result, str, len); + result[len] = '/'; + result[len + 1] = '\0'; + return result; +} + char *strlpart(char *txt, int maxlen) { char *result; diff --git a/ui-shared.c b/ui-shared.c index d4fb3d9..e9c1977 100644 --- a/ui-shared.c +++ b/ui-shared.c @@ -56,7 +56,7 @@ char *cgit_hosturl() char *cgit_rooturl() { if (ctx.cfg.virtual_root) - return fmt("%s/", ctx.cfg.virtual_root); + return ctx.cfg.virtual_root; else return ctx.cfg.script_name; } @@ -64,7 +64,7 @@ char *cgit_rooturl() char *cgit_repourl(const char *reponame) { if (ctx.cfg.virtual_root) { - return fmt("%s/%s/", ctx.cfg.virtual_root, reponame); + return fmt("%s%s/", ctx.cfg.virtual_root, reponame); } else { return fmt("?r=%s", reponame); } @@ -77,7 +77,7 @@ char *cgit_fileurl(const char *reponame, const char *pagename, char *delim; if (ctx.cfg.virtual_root) { - tmp = fmt("%s/%s/%s/%s", ctx.cfg.virtual_root, reponame, + tmp = fmt("%s%s/%s/%s", ctx.cfg.virtual_root, reponame, pagename, (filename ? filename:"")); delim = "?"; } else { @@ -125,11 +125,9 @@ static void site_url(const char *page, const char *search, const char *sort, int { char *delim = "?"; - if (ctx.cfg.virtual_root) { + if (ctx.cfg.virtual_root) html_attr(ctx.cfg.virtual_root); - if (ctx.cfg.virtual_root[strlen(ctx.cfg.virtual_root) - 1] != '/') - html("/"); - } else + else html(ctx.cfg.script_name); if (page) { @@ -200,8 +198,6 @@ static char *repolink(const char *title, const char *class, const char *page, html(" href='"); if (ctx.cfg.virtual_root) { html_url_path(ctx.cfg.virtual_root); - if (ctx.cfg.virtual_root[strlen(ctx.cfg.virtual_root) - 1] != '/') - html("/"); html_url_path(ctx.repo->url); if (ctx.repo->url[strlen(ctx.repo->url) - 1] != '/') html("/"); -- 1.8.2.540.gf023cfe _______________________________________________ cgit mailing list cgit@hjemli.net http://hjemli.net/mailman/listinfo/cgit