- From:
- Omar Polo <op@omarpolo.com>
- Subject:
- gotwebd: some html fixes
- To:
- gameoftrees@openbsd.org
- Date:
- Fri, 19 Aug 2022 19:30:59 +0200
Download raw body.
Thread
gotwebd: some html fixes
Download raw body.
gotwebd: some html fixes
as all the tragic stories goes, it all started with a simple goal of
making the W3C validator happy and endend up in pain and (html)
escaping.
The w3c validator complained about three things:
- invalid target=_sotd in a link
- unexpected `alt' attribute on some links
- missing lang on the html tag.
the last one is questionable: we don't really know in what language
the commits will be (especially when they don't need to be in the same
language!) but still i think that since the whole UI is in english
there's not much harm in setting lang=en.
Then, I saw a few places where we forgot to escape some strings and
re-cheked all the fcgi_printf calls.
So, I'd like to propose diff below. it's actually two commit
together, I'm bundling them here because it's easier. It fixes the
issue reported by the w3c validator and escapes all the potential
unsafe strings I've found.
There's still a bit that's missing and it's properly URL-escape links,
which is different from the html escaping, but i'd like to address
that in a follow up commit. gotwebd needs to learn how to
percent-decode querystrings too.
I have this diff on my instance, now I can render 'funny' trees with
files like '<script>' :)
https://git.omarpolo.com/?index_page=0&path=testing.git&action=tree&commit=78397ac198dbef68c371cef13a4fb986ff93fe34&headref=HEAD
(note that the first two links don't work because of the escaping)
thoughts/ok?
diff refs/remotes/got/main refs/heads/main
commit - 01498c42e0fc9fb6355312a236656cf2f36cebc0
commit + 0e9860257eadd668942affe9f2846a6e09603e3b
blob - 35735937cc54916d0fa7248397a0a66248c0c510
blob + 18dca123b78589b4efb8482bc4caa54e1aae7176
--- gotwebd/got_operations.c
+++ gotwebd/got_operations.c
@@ -830,7 +830,7 @@ got_output_repo_tree(struct request *c)
struct got_tree_object *tree = NULL;
struct repo_dir *repo_dir = t->repo_dir;
const char *name, *index_page_str, *folder;
- char *id_str = NULL;
+ char *id_str = NULL, *escaped_name;
char *path = NULL, *in_repo_path = NULL, *modestr = NULL;
int nentries, i, r;
@@ -920,8 +920,12 @@ got_output_repo_tree(struct request *c)
}
}
+ name = got_tree_entry_get_name(te);
+ error = gotweb_escape_html(&escaped_name, name);
+ if (error)
+ goto done;
+
if (S_ISDIR(mode)) {
- name = got_tree_entry_get_name(te);
r = fcgi_printf(c,
"<div class='tree_wrapper'>\n"
"<div class='tree_line'>"
@@ -932,11 +936,10 @@ got_output_repo_tree(struct request *c)
"<div class='tree_line_blank'> </div>\n"
"</div>\n", /* .tree_wrapper */
index_page_str, qs->path, rc->commit_id,
- folder, name, name, modestr);
+ folder, name, escaped_name, modestr);
if (r == -1)
goto done;
} else {
- name = got_tree_entry_get_name(te);
r = fcgi_printf(c,
"<div class='tree_wrapper'>\n"
"<div class='tree_line'>"
@@ -952,7 +955,7 @@ got_output_repo_tree(struct request *c)
"</div>\n" /* .tree_line_blank */
"</div>\n", /* .tree_wrapper */
index_page_str, qs->path, rc->commit_id,
- folder, name, name, modestr,
+ folder, name, escaped_name, modestr,
index_page_str, qs->path, rc->commit_id,
folder, name,
index_page_str, qs->path, rc->commit_id,
blob - 9358e442cb79338b33110402f61e9bd695a9690d
blob + 4dfa5656c52bd7eff6cdc8b2196476bb8e664581
--- gotwebd/gotweb.c
+++ gotwebd/gotweb.c
@@ -655,7 +655,7 @@ gotweb_render_header(struct request *c)
strlcpy(droot, "/", sizeof(droot));
r = fcgi_printf(c, "<!doctype html>\n"
- "<html>\n"
+ "<html lang=\"en\">\n"
"<head>\n"
"<title>%s</title>\n"
"<meta charset='utf-8' />\n"
@@ -676,14 +676,14 @@ gotweb_render_header(struct request *c)
"<div id='gw_body'>\n"
"<div id='header'>\n"
"<div id='got_link'>"
- "<a href='%s' target='_sotd'>"
+ "<a href='%s' target='_blank'>"
"<img src='%s%s' alt='logo' id='logo' />"
"</a>\n"
"</div>\n" /* #got_link */
"</div>\n" /* #header */
"<div id='site_path'>\n"
"<div id='site_link'>\n"
- "<a href='/%s?index_page=%d' alt='sitelink'>%s</a>",
+ "<a href='/%s?index_page=%d'>%s</a>",
srv->site_name,
droot, srv->custom_css,
srv->logo_url,
@@ -695,8 +695,8 @@ gotweb_render_header(struct request *c)
if (qs != NULL) {
if (qs->path != NULL) {
r = fcgi_printf(c, " / "
- "<a href='/%s?index_page=%d&path=%s&action=summary'"
- " alt='summlink'>%s</a>",
+ "<a href='/%s?index_page=%d&path=%s&action=summary'>"
+ "%s</a>",
c->document_root, qs->index_page, qs->path,
qs->path);
if (r == -1)
@@ -1136,7 +1136,7 @@ gotweb_render_blame(struct request *c)
const struct got_error *error = NULL;
struct transport *t = c->t;
struct repo_commit *rc = NULL;
- char *age = NULL;
+ char *age = NULL, *msg = NULL;
int r;
error = got_get_repo_commits(c, 1);
@@ -1148,6 +1148,9 @@ gotweb_render_blame(struct request *c)
error = gotweb_get_time_str(&age, rc->committer_time, TM_LONG);
if (error)
goto done;
+ error = gotweb_escape_html(&msg, rc->commit_msg);
+ if (error)
+ goto done;
r = fcgi_printf(c, "<div id='blame_title_wrapper'>\n"
"<div id='blame_title'>Blame</div>\n"
@@ -1164,7 +1167,7 @@ gotweb_render_blame(struct request *c)
"<div class='dotted_line'></div>\n"
"<div id='blame'>\n",
age ? age : "",
- rc->commit_msg);
+ msg);
if (r == -1)
goto done;
@@ -1175,6 +1178,7 @@ gotweb_render_blame(struct request *c)
fcgi_printf(c, "</div>\n" /* #blame */
"</div>\n"); /* #blame_content */
done:
+ free(msg);
return error;
}
@@ -1189,7 +1193,7 @@ gotweb_render_briefs(struct request *c)
struct repo_dir *repo_dir = t->repo_dir;
const char *index_page_str;
char *smallerthan, *newline;
- char *age = NULL;
+ char *age = NULL, *author = NULL, *msg = NULL;
int r;
index_page_str = qs->index_page_str ? qs->index_page_str : "";
@@ -1222,22 +1226,34 @@ gotweb_render_briefs(struct request *c)
if (newline)
*newline = '\0';
+ error = gotweb_escape_html(&author, rc->author);
+ if (error)
+ goto done;
+ error = gotweb_escape_html(&msg, rc->commit_msg);
+ if (error)
+ goto done;
+
r = fcgi_printf(c, "<div class='briefs_age'>%s</div>\n"
"<div class='briefs_author'>%s</div>\n"
"<div class='briefs_log'>"
"<a href='?index_page=%s&path=%s&action=diff&commit=%s"
"&headref=%s'>%s</a>",
age ? age : "",
- rc->author,
+ author,
index_page_str, repo_dir->name, rc->commit_id, qs->headref,
- rc->commit_msg);
+ msg);
if (r == -1)
goto done;
if (rc->refs_str) {
+ char *refs;
+
+ error = gotweb_escape_html(&refs, rc->refs_str);
+ if (error)
+ goto done;
r = fcgi_printf(c,
- " <span class='refs_str'>(%s)</span>",
- rc->refs_str);
+ " <span class='refs_str'>(%s)</span>", refs);
+ free(refs);
if (r == -1)
goto done;
}
@@ -1261,6 +1277,10 @@ gotweb_render_briefs(struct request *c)
free(age);
age = NULL;
+ free(author);
+ author = NULL;
+ free(msg);
+ msg = NULL;
}
if (t->next_id || t->prev_id) {
@@ -1271,6 +1291,8 @@ gotweb_render_briefs(struct request *c)
fcgi_printf(c, "</div>\n"); /* #briefs_content */
done:
free(age);
+ free(author);
+ free(msg);
return error;
}
@@ -1284,7 +1306,7 @@ gotweb_render_commits(struct request *c)
struct querystring *qs = t->qs;
struct repo_dir *repo_dir = t->repo_dir;
const char *index_page_str;
- char *age = NULL, *author = NULL;
+ char *age = NULL, *author = NULL, *msg = NULL;
int r;
index_page_str = qs->index_page_str ? qs->index_page_str : "";
@@ -1307,6 +1329,9 @@ gotweb_render_commits(struct request *c)
error = gotweb_escape_html(&author, rc->author);
if (error)
goto done;
+ error = gotweb_escape_html(&msg, rc->commit_msg);
+ if (error)
+ goto done;
r = fcgi_printf(c, "<div class='commits_header_wrapper'>\n"
"<div class='commits_header'>\n"
@@ -1321,9 +1346,9 @@ gotweb_render_commits(struct request *c)
"<div class='dotted_line'></div>\n"
"<div class='commit'>\n%s</div>\n",
rc->commit_id,
- author ? author : "",
+ author,
age ? age : "",
- rc->commit_msg);
+ msg);
if (r == -1)
goto done;
@@ -1344,6 +1369,8 @@ gotweb_render_commits(struct request *c)
age = NULL;
free(author);
author = NULL;
+ free(msg);
+ msg = NULL;
}
if (t->next_id || t->prev_id) {
@@ -1354,6 +1381,8 @@ gotweb_render_commits(struct request *c)
fcgi_printf(c, "</div>\n"); /* .commits_content */
done:
free(age);
+ free(author);
+ free(msg);
return error;
}
@@ -1387,12 +1416,13 @@ gotweb_render_branches(struct request *c)
goto done;
TAILQ_FOREACH(re, &refs, entry) {
- char *refname = NULL;
+ const char *refname = NULL;
+ char *escaped_refname = NULL;
if (got_ref_is_symbolic(re->ref))
continue;
- refname = strdup(got_ref_get_name(re->ref));
+ refname = got_ref_get_name(re->ref);
if (refname == NULL) {
error = got_error_from_errno("strdup");
goto done;
@@ -1407,6 +1437,9 @@ gotweb_render_branches(struct request *c)
if (strncmp(refname, "refs/heads/", 11) == 0)
refname += 11;
+ error = gotweb_escape_html(&escaped_refname, refname);
+ if (error)
+ goto done;
r = fcgi_printf(c, "<div class='branches_wrapper'>\n"
"<div class='branches_age'>%s</div>\n"
@@ -1431,10 +1464,11 @@ gotweb_render_branches(struct request *c)
"</div>\n", /* .branches_wrapper */
age ? age : "",
index_page_str, qs->path, refname,
- refname,
+ escaped_refname,
index_page_str, qs->path, refname,
index_page_str, qs->path, refname,
index_page_str, qs->path, refname);
+ free(escaped_refname);
if (r == -1)
goto done;
@@ -1453,7 +1487,7 @@ gotweb_render_tree(struct request *c)
const struct got_error *error = NULL;
struct transport *t = c->t;
struct repo_commit *rc = NULL;
- char *age = NULL;
+ char *age = NULL, *msg = NULL;
int r;
error = got_get_repo_commits(c, 1);
@@ -1466,6 +1500,10 @@ gotweb_render_tree(struct request *c)
if (error)
goto done;
+ error = gotweb_escape_html(&msg, rc->commit_msg);
+ if (error)
+ goto done;
+
r = fcgi_printf(c, "<div id='tree_title_wrapper'>\n"
"<div id='tree_title'>Tree</div>\n"
"</div>\n" /* #tree_title_wrapper */
@@ -1484,7 +1522,7 @@ gotweb_render_tree(struct request *c)
"<div id='tree'>\n",
rc->tree_id,
age ? age : "",
- rc->commit_msg);
+ msg);
if (r == -1)
goto done;
@@ -1495,6 +1533,7 @@ gotweb_render_tree(struct request *c)
fcgi_printf(c, "</div>\n"); /* #tree */
fcgi_printf(c, "</div>\n"); /* #tree_content */
done:
+ free(msg);
return error;
}
@@ -1504,7 +1543,7 @@ gotweb_render_diff(struct request *c)
const struct got_error *error = NULL;
struct transport *t = c->t;
struct repo_commit *rc = NULL;
- char *age = NULL, *author = NULL;
+ char *age = NULL, *author = NULL, *msg = NULL;
int r;
error = got_get_repo_commits(c, 1);
@@ -1519,6 +1558,9 @@ gotweb_render_diff(struct request *c)
error = gotweb_escape_html(&author, rc->author);
if (error)
goto done;
+ error = gotweb_escape_html(&msg, rc->commit_msg);
+ if (error)
+ goto done;
r = fcgi_printf(c, "<div id='diff_title_wrapper'>\n"
"<div id='diff_title'>Commit Diff</div>\n"
@@ -1545,9 +1587,9 @@ gotweb_render_diff(struct request *c)
rc->parent_id, rc->commit_id,
rc->commit_id,
rc->tree_id,
- author ? author : "",
+ author,
age ? age : "",
- rc->commit_msg);
+ msg);
if (r == -1)
goto done;
@@ -1560,6 +1602,7 @@ gotweb_render_diff(struct request *c)
done:
free(age);
free(author);
+ free(msg);
return error;
}
@@ -1578,7 +1621,7 @@ gotweb_render_summary(struct request *c)
r = fcgi_printf(c,
"<div id='description_title'>Description:</div>\n"
"<div id='description'>%s</div>\n",
- t->repo_dir->description);
+ t->repo_dir->description ? t->repo_dir->description : "");
if (r == -1)
goto done;
}
@@ -1587,7 +1630,7 @@ gotweb_render_summary(struct request *c)
r = fcgi_printf(c,
"<div id='repo_owner_title'>Owner:</div>\n"
"<div id='repo_owner'>%s</div>\n",
- t->repo_dir->owner);
+ t->repo_dir->owner ? t->repo_dir->owner : "");
if (r == -1)
goto done;
}
@@ -1639,7 +1682,7 @@ gotweb_render_tag(struct request *c)
const struct got_error *error = NULL;
struct repo_tag *rt = NULL;
struct transport *t = c->t;
- char *age = NULL, *author = NULL;
+ char *tagname = NULL, *age = NULL, *author = NULL, *msg = NULL;
error = got_get_repo_tags(c, 1);
if (error)
@@ -1659,9 +1702,15 @@ gotweb_render_tag(struct request *c)
error = gotweb_escape_html(&author, rt->tagger);
if (error)
goto done;
+ error = gotweb_escape_html(&msg, rt->commit_msg);
+ if (error)
+ goto done;
if (strncmp(rt->tag_name, "refs/", 5) == 0)
rt->tag_name += 5;
+ error = gotweb_escape_html(&tagname, rt->tag_name);
+ if (error)
+ goto done;
fcgi_printf(c, "<div id='tags_title_wrapper'>\n"
"<div id='tags_title'>Tag</div>\n"
@@ -1683,15 +1732,16 @@ gotweb_render_tag(struct request *c)
"<div id='tag_commit'>\n%s</div>"
"</div>", /* tag_header_wrapper */
rt->commit_id,
- rt->tag_name,
- author ? author : "",
+ tagname,
+ author,
age ? age : "",
- rt->commit_msg,
+ msg,
rt->tag_commit);
done:
free(age);
free(author);
+ free(msg);
return error;
}
@@ -1705,8 +1755,7 @@ gotweb_render_tags(struct request *c)
struct querystring *qs = t->qs;
struct repo_dir *repo_dir = t->repo_dir;
const char *index_page_str;
- char *newline;
- char *age = NULL;
+ char *age = NULL, *tagname = NULL, *msg = NULL, *newline;
int r, commit_found = 0;
index_page_str = qs->index_page_str ? qs->index_page_str : "";
@@ -1746,11 +1795,17 @@ gotweb_render_tags(struct request *c)
if (strncmp(rt->tag_name, "refs/tags/", 10) == 0)
rt->tag_name += 10;
+ error = gotweb_escape_html(&tagname, rt->tag_name);
+ if (error)
+ goto done;
if (rt->tag_commit != NULL) {
newline = strchr(rt->tag_commit, '\n');
if (newline)
*newline = '\0';
+ error = gotweb_escape_html(&msg, rt->tag_commit);
+ if (error)
+ goto done;
}
r = fcgi_printf(c, "<div class='tag_age'>%s</div>\n"
@@ -1773,9 +1828,9 @@ gotweb_render_tags(struct request *c)
"</div>\n" /* .navs_wrapper */
"<div class='dotted_line'></div>\n",
age ? age : "",
- rt->tag_name,
+ tagname,
index_page_str, repo_dir->name, rt->commit_id,
- rt->tag_commit ? rt->tag_commit : "",
+ msg ? msg : "",
index_page_str, repo_dir->name, rt->commit_id,
index_page_str, repo_dir->name, rt->commit_id,
index_page_str, repo_dir->name, rt->commit_id);
@@ -1784,6 +1839,10 @@ gotweb_render_tags(struct request *c)
free(age);
age = NULL;
+ free(tagname);
+ tagname = NULL;
+ free(msg);
+ msg = NULL;
}
if (t->next_id || t->prev_id) {
error = gotweb_render_navs(c);
@@ -1793,6 +1852,8 @@ gotweb_render_tags(struct request *c)
fcgi_printf(c, "</div>\n"); /* #tags_content */
done:
free(age);
+ free(tagname);
+ free(msg);
return error;
}
gotwebd: some html fixes