Download raw body.
gotwebd: percent-encode querystrings
On Tue, Sep 06, 2022 at 05:12:14PM +0200, Omar Polo wrote:
> On 2022/09/03 11:27:58 +0200, Omar Polo <op@omarpolo.com> wrote:
> > TL;DR: wanna break gotwebd?
> >
> > $ touch "' onclick='alert(\"p0wn3d\");' foo='"
> > $ got add -R .
> > $ got commit -m ...
> > $ got send
> >
> > enjoy!
> >
> > (this doesn't work on modern browsers due to the CSP, but gives the
> > idea.)
> >
> >
> > Longer version: gotwebd doesn't encode the arguments in the generated
> > URLs. This is a companion diff to the other diff i've sent ("gotwebd:
> > urldecode querystring") and deals with how URLs are generated.
> >
> > Some URL parameters needs to be encoded as they contain data that may
> > otherwise break the HTML. Among these `folder', `file' and `path'.
> > Instead of making the current URL construction even more complex, I
> > decided to take it as a chance to refactor a bit how we generate URLs.
> >
> > Diff below introduces a new helper: `gotweb_link'. It takes the
> > request, a struct gotweb_url that represent the request, followed by a
> > printf-format string and arguments. It makes gotwebd gain a few lines
> > of codes because now the various fcgi_printf calls needs to be
> > splitted, but otherwise I think it's an improvement.
> >
> > What do y'all think? Is it OK to use the syntax
> >
> > r = gotweb_link(c, &(struct gotweb_url){
> > .index_page = -1,
> > .action = DIFF,
> > .page = -1,
> > .path = repo_dir->name,
> > .commit = bline->id_str,
> > }, "%.8s", bline->id_str);
> >
> > to pass an anounymous struct pointer?
>
> here's a rebased and slightly improved diff. it makes gotwebd gains a
> few lines of code but I think it's better than revisit every function
> where we print a link and allocate yet another local string that we
> might forget to free and making the output functions even more complex
> to follow. it also centralize how we generate URLs, hopefully making
> the life easier in the future if we want to change things.
>
> the changes to the previous version are:
>
> - escape the `headref' parametr too
> - use a consistent ordering of the fields
> - add a comment before the gotweb_url struct
This breaks next and previous links.
>
> diff refs/heads/main refs/heads/%enc
> commit - 8ee99f946108c3442fcc98fa53ecc9264ed5d947
> commit + 34cef9dcccb49096963b96eac94f884f62cd9a45
> blob - e77b8424231bf88c153a32dd90a19292c41dc39d
> blob + 04e9e72ece31adeacc0aef6f6fb7cffbaace078a
> --- gotwebd/fcgi.c
> +++ gotwebd/fcgi.c
> @@ -288,16 +288,12 @@ fcgi_timeout(int fd, short events, void *arg)
> }
>
> int
> -fcgi_printf(struct request *c, const char *fmt, ...)
> +fcgi_vprintf(struct request *c, const char *fmt, va_list ap)
> {
> - va_list ap;
> char *str;
> int r;
>
> - va_start(ap, fmt);
> r = vasprintf(&str, fmt, ap);
> - va_end(ap);
> -
> if (r == -1) {
> log_warn("%s: asprintf", __func__);
> return -1;
> @@ -309,6 +305,19 @@ fcgi_printf(struct request *c, const char *fmt, ...)
> }
>
> int
> +fcgi_printf(struct request *c, const char *fmt, ...)
> +{
> + va_list ap;
> + int r;
> +
> + va_start(ap, fmt);
> + r = fcgi_vprintf(c, fmt, ap);
> + va_end(ap);
> +
> + return r;
> +}
> +
> +int
> fcgi_gen_binary_response(struct request *c, const uint8_t *data, int len)
> {
> int r;
> blob - 04dc980392274b236587daf89d34a45c992b64c1
> blob + 5211e085ce0097066a9ec6cb94c843ffc952f29f
> --- gotwebd/got_operations.c
> +++ gotwebd/got_operations.c
> @@ -810,7 +810,7 @@ got_output_repo_tree(struct request *c)
> struct got_reflist_head refs;
> struct got_tree_object *tree = NULL;
> struct repo_dir *repo_dir = t->repo_dir;
> - const char *name, *index_page_str, *folder;
> + const char *name, *folder;
> char *escaped_name = NULL, *path = NULL;
> int nentries, i, r;
>
> @@ -849,7 +849,6 @@ got_output_repo_tree(struct request *c)
>
> nentries = got_object_tree_get_nentries(tree);
>
> - index_page_str = qs->index_page_str ? qs->index_page_str : "";
> folder = qs->folder ? qs->folder : "";
>
> for (i = 0; i < nentries; i++) {
> @@ -877,42 +876,76 @@ got_output_repo_tree(struct request *c)
> goto done;
>
> if (S_ISDIR(mode)) {
> - r = fcgi_printf(c,
> - "<div class='tree_wrapper'>\n"
> - "<div class='tree_line'>"
> - "<a class='diff_directory' "
> - "href='?index_page=%s&path=%s&action=tree"
> - "&commit=%s&folder=%s/%s'>%s%s</a>"
> - "</div>\n" /* .tree_line */
> + struct gotweb_url url = {
> + .index_page = -1,
> + .page = -1,
> + .action = TREE,
> + .commit = rc->commit_id,
> + .path = qs->path,
> + /* `folder' is filled later */
> + };
> + char *path = NULL;
> +
> + if (fcgi_printf(c,"<div class='tree_wrapper'>\n"
> + "<div class='tree_line'>") == -1)
> + goto done;
> +
> + if (asprintf(&path, "%s/%s", folder, name) == -1) {
> + error = got_error_from_errno("asprintf");
> + goto done;
> + }
> + url.folder = path;
> + r = gotweb_link(c, &url, "%s%s", escaped_name,
> + modestr);
> + free(path);
> + if (r == -1)
> + goto done;
> +
> + if (fcgi_printf(c, "</div>\n" /* .tree_line */
> "<div class='tree_line_blank'> </div>\n"
> - "</div>\n", /* .tree_wrapper */
> - index_page_str, qs->path, rc->commit_id,
> - folder, name, escaped_name, modestr);
> - if (r == -1)
> + "</div>\n") == -1)
> goto done;
> } else {
> - r = fcgi_printf(c,
> - "<div class='tree_wrapper'>\n"
> - "<div class='tree_line'>"
> - "<a href='?index_page=%s&path=%s&action=blob"
> - "&commit=%s&folder=%s&file=%s'>%s%s</a>"
> - "</div>\n" /* .tree_line */
> - "<div class='tree_line_blank'>"
> - "<a href='?index_page=%s&path=%s&action=commits"
> - "&commit=%s&folder=%s&file=%s'>commits</a>\n"
> - " | \n"
> - "<a href='?index_page=%s&path=%s&action=blame"
> - "&commit=%s&folder=%s&file=%s'>blame</a>\n"
> - "</div>\n" /* .tree_line_blank */
> - "</div>\n", /* .tree_wrapper */
> - index_page_str, qs->path, rc->commit_id,
> - folder, name, escaped_name, modestr,
> - index_page_str, qs->path, rc->commit_id,
> - folder, name,
> - index_page_str, qs->path, rc->commit_id,
> - folder, name);
> + struct gotweb_url url = {
> + .index_page = -1,
> + .page = -1,
> + .path = qs->path,
> + .commit = rc->commit_id,
> + .folder = folder,
> + .file = name,
> + };
> +
> + if (fcgi_printf(c, "<div class='tree_wrapper'>\n"
> + "<div class='tree_line'>") == -1)
> + goto done;
> +
> + url.action = BLOB;
> + r = gotweb_link(c, &url, "%s%s", escaped_name,
> + modestr);
> if (r == -1)
> goto done;
> +
> + if (fcgi_printf(c, "</div>\n" /* .tree_line */
> + "<div class='tree_line_blank'>") == -1)
> + goto done;
> +
> + url.action = COMMITS;
> + r = gotweb_link(c, &url, "commits");
> + if (r == -1)
> + goto done;
> +
> + if (fcgi_printf(c, " | ") == -1)
> + goto done;
> +
> + url.action = BLAME;
> + r = gotweb_link(c, &url, "blame");
> + if (r == -1)
> + goto done;
> +
> + if (fcgi_printf(c,
> + "</div>\n" /* .tree_line_blank */
> + "</div>\n") == -1) /* .tree_wrapper */
> + goto done;
> }
> free(escaped_name);
> escaped_name = NULL;
> @@ -1069,9 +1102,7 @@ got_gotweb_blame_cb(void *arg, int nlines, int lineno,
> struct blame_line *bline;
> struct request *c = a->c;
> struct transport *t = c->t;
> - struct querystring *qs = t->qs;
> struct repo_dir *repo_dir = t->repo_dir;
> - const char *index_page_str;
> char *line = NULL, *eline = NULL;
> size_t linesize = 0;
> off_t offset;
> @@ -1121,8 +1152,6 @@ got_gotweb_blame_cb(void *arg, int nlines, int lineno,
> goto done;
> }
>
> - index_page_str = qs->index_page_str ? qs->index_page_str : "";
> -
> while (a->lineno_cur <= a->nlines && bline->annotated) {
> char *smallerthan, *at, *nl, *committer;
> size_t len;
> @@ -1152,19 +1181,28 @@ got_gotweb_blame_cb(void *arg, int nlines, int lineno,
> if (err)
> goto done;
>
> - r = fcgi_printf(c, "<div class='blame_wrapper'>"
> + if (fcgi_printf(c, "<div class='blame_wrapper'>"
> "<div class='blame_number'>%.*d</div>"
> - "<div class='blame_hash'>"
> - "<a href='?index_page=%s&path=%s&action=diff&commit=%s'>"
> - "%.8s</a>"
> + "<div class='blame_hash'>",
> + a->nlines_prec, a->lineno_cur) == -1)
> + goto done;
> +
> + r = gotweb_link(c, &(struct gotweb_url){
> + .action = DIFF,
> + .index_page = -1,
> + .page = -1,
> + .path = repo_dir->name,
> + .commit = bline->id_str,
> + }, "%.8s", bline->id_str);
> + if (r == -1)
> + goto done;
> +
> + r = fcgi_printf(c,
> "</div>"
> "<div class='blame_date'>%s</div>"
> "<div class='blame_author'>%s</div>"
> "<div class='blame_code'>%s</div>"
> "</div>", /* .blame_wrapper */
> - a->nlines_prec, a->lineno_cur,
> - index_page_str, repo_dir->name, bline->id_str,
> - bline->id_str,
> bline->datebuf,
> committer,
> eline);
> blob - 1312e2683bc2b1ff818d0c837d1b1e4ce670ab62
> blob + 5e13cb4f5f40682282ea17f7ab4440076a52815b
> --- gotwebd/gotweb.c
> +++ gotwebd/gotweb.c
> @@ -694,6 +694,7 @@ gotweb_render_content_type_file(struct request *c, con
> static const struct got_error *
> gotweb_render_header(struct request *c)
> {
> + const struct got_error *err = NULL;
> struct server *srv = c->srv;
> struct querystring *qs = c->t->qs;
> int r;
> @@ -743,10 +744,21 @@ gotweb_render_header(struct request *c)
>
> if (qs != NULL) {
> if (qs->path != NULL) {
> - r = fcgi_printf(c, " / "
> - "<a href='?index_page=%d&path=%s&action=summary'>"
> - "%s</a>",
> - qs->index_page, qs->path, qs->path);
> + char *epath;
> +
> + if (fcgi_printf(c, " / ") == -1)
> + goto done;
> +
> + err = gotweb_escape_html(&epath, qs->path);
> + if (err)
> + return err;
> + r = gotweb_link(c, &(struct gotweb_url){
> + .action = SUMMARY,
> + .index_page = -1,
> + .page = -1,
> + .path = qs->path,
> + }, "%s", epath);
> + free(epath);
> if (r == -1)
> goto done;
> }
> @@ -826,8 +838,7 @@ gotweb_render_navs(struct request *c)
> struct transport *t = c->t;
> struct querystring *qs = t->qs;
> struct server *srv = c->srv;
> - char *nhref = NULL, *phref = NULL;
> - int r, disp = 0;
> + int r;
>
> r = fcgi_printf(c, "<div id='np_wrapper'>\n<div id='nav_prev'>\n");
> if (r == -1)
> @@ -836,140 +847,134 @@ gotweb_render_navs(struct request *c)
> switch(qs->action) {
> case INDEX:
> if (qs->index_page > 0) {
> - if (asprintf(&phref, "index_page=%d",
> - qs->index_page - 1) == -1) {
> - error = got_error_from_errno2("%s: asprintf",
> - __func__);
> - goto done;
> - }
> - disp = 1;
> + struct gotweb_url url = {
> + .action = -1,
> + .index_page = qs->index_page - 1,
> + .page = -1,
> + };
> +
> + r = gotweb_link(c, &url, "Previous");
> }
> break;
> case BRIEFS:
> if (t->prev_id && qs->commit != NULL &&
> strcmp(qs->commit, t->prev_id) != 0) {
> - if (asprintf(&phref, "index_page=%d&path=%s&page=%d"
> - "&action=briefs&commit=%s&headref=%s",
> - qs->index_page, qs->path, qs->page - 1, t->prev_id,
> - qs->headref) == -1) {
> - error = got_error_from_errno2("%s: asprintf",
> - __func__);
> - goto done;
> - }
> - disp = 1;
> + struct gotweb_url url = {
> + .action = BRIEFS,
> + .index_page = -1,
> + .page = qs->page - 1,
> + .path = qs->path,
> + .commit = t->prev_id,
> + .headref = qs->headref,
> + };
> +
> + r = gotweb_link(c, &url, "Previous");
> }
> break;
> case COMMITS:
> if (t->prev_id && qs->commit != NULL &&
> strcmp(qs->commit, t->prev_id) != 0) {
> - if (asprintf(&phref, "index_page=%d&path=%s&page=%d"
> - "&action=commits&commit=%s&headref=%s&folder=%s"
> - "&file=%s",
> - qs->index_page, qs->path, qs->page - 1, t->prev_id,
> - qs->headref, qs->folder ? qs->folder : "",
> - qs->file ? qs->file : "") == -1) {
> - error = got_error_from_errno2("%s: asprintf",
> - __func__);
> - goto done;
> - }
> - disp = 1;
> + struct gotweb_url url = {
> + .action = COMMIT,
> + .index_page = -1,
> + .page = qs->page - 1,
> + .path = qs->path,
> + .commit = t->prev_id,
> + .headref = qs->headref,
> + .folder = qs->folder,
> + .file = qs->file,
> + };
> +
> + r = gotweb_link(c, &url, "Previous");
> }
> break;
> case TAGS:
> if (t->prev_id && qs->commit != NULL &&
> strcmp(qs->commit, t->prev_id) != 0) {
> - if (asprintf(&phref, "index_page=%d&path=%s&page=%d"
> - "&action=tags&commit=%s&headref=%s",
> - qs->index_page, qs->path, qs->page - 1, t->prev_id,
> - qs->headref) == -1) {
> - error = got_error_from_errno2("%s: asprintf",
> - __func__);
> - goto done;
> - }
> - disp = 1;
> + struct gotweb_url url = {
> + .action = TAGS,
> + .index_page = -1,
> + .page = qs->page - 1,
> + .path = qs->path,
> + .commit = t->prev_id,
> + .headref = qs->headref,
> + };
> +
> + r = gotweb_link(c, &url, "Previous");
> }
> break;
> - default:
> - disp = 0;
> - break;
> }
>
> - if (disp) {
> - r = fcgi_printf(c, "<a href='?%s'>Previous</a>", phref);
> - if (r == -1)
> - goto done;
> - }
> + if (r == -1)
> + goto done;
>
> r = fcgi_printf(c, "</div>\n" /* #nav_prev */
> "<div id='nav_next'>");
> if (r == -1)
> goto done;
>
> - disp = 0;
> switch(qs->action) {
> case INDEX:
> if (t->next_disp == srv->max_repos_display &&
> t->repos_total != (qs->index_page + 1) *
> srv->max_repos_display) {
> - if (asprintf(&nhref, "index_page=%d",
> - qs->index_page + 1) == -1) {
> - error = got_error_from_errno2("%s: asprintf",
> - __func__);
> - goto done;
> - }
> - disp = 1;
> + struct gotweb_url url = {
> + .action = -1,
> + .index_page = qs->index_page + 1,
> + .page = -1,
> + };
> +
> + r = gotweb_link(c, &url, "Next");
> }
> break;
> case BRIEFS:
> if (t->next_id) {
> - if (asprintf(&nhref, "index_page=%d&path=%s&page=%d"
> - "&action=briefs&commit=%s&headref=%s",
> - qs->index_page, qs->path, qs->page + 1, t->next_id,
> - qs->headref) == -1) {
> - error = got_error_from_errno2("%s: asprintf",
> - __func__);
> - goto done;
> - }
> - disp = 1;
> + struct gotweb_url url = {
> + .action = BRIEFS,
> + .index_page = -1,
> + .page = qs->page + 1,
> + .path = qs->path,
> + .commit = t->next_id,
> + .headref = qs->headref,
> + };
> +
> + r = gotweb_link(c, &url, "Next");
> }
> break;
> case COMMITS:
> if (t->next_id) {
> - if (asprintf(&nhref, "index_page=%d&path=%s&page=%d"
> - "&action=commits&commit=%s&headref=%s&folder=%s"
> - "&file=%s",
> - qs->index_page, qs->path, qs->page + 1, t->next_id,
> - qs->headref, qs->folder ? qs->folder : "",
> - qs->file ? qs->file : "") == -1) {
> - error = got_error_from_errno2("%s: asprintf",
> - __func__);
> - goto done;
> - }
> - disp = 1;
> + struct gotweb_url url = {
> + .action = COMMIT,
> + .index_page = -1,
> + .page = qs->page + 1,
> + .path = qs->path,
> + .commit = t->next_id,
> + .headref = qs->headref,
> + .folder = qs->folder,
> + .file = qs->file,
> + };
> +
> + r = gotweb_link(c, &url, "Next");
> }
> break;
> case TAGS:
> if (t->next_id) {
> - if (asprintf(&nhref, "index_page=%d&path=%s&page=%d"
> - "&action=tags&commit=%s&headref=%s",
> - qs->index_page, qs->path, qs->page + 1, t->next_id,
> - qs->headref) == -1) {
> - error = got_error_from_errno2("%s: asprintf",
> - __func__);
> - goto done;
> - }
> - disp = 1;
> + struct gotweb_url url = {
> + .action = TAGS,
> + .index_page = -1,
> + .page = qs->page + 1,
> + .path = qs->path,
> + .commit = t->next_id,
> + .headref = qs->headref,
> + };
> +
> + r = gotweb_link(c, &url, "Next");
> }
> break;
> - default:
> - disp = 0;
> - break;
> }
> - if (disp) {
> - r = fcgi_printf(c, "<a href='?%s'>Next</a>", nhref);
> - if (r == -1)
> - goto done;
> - }
> + if (r == -1)
> + goto done;
> +
> fcgi_printf(c, "</div>\n"); /* #nav_next */
> fcgi_printf(c, "</div>\n"); /* #np_wrapper */
> done:
> @@ -977,8 +982,6 @@ done:
> t->next_id = NULL;
> free(t->prev_id);
> t->prev_id = NULL;
> - free(phref);
> - free(nhref);
> return error;
> }
>
> @@ -992,14 +995,11 @@ gotweb_render_index(struct request *c)
> struct repo_dir *repo_dir = NULL;
> DIR *d;
> struct dirent **sd_dent = NULL;
> - const char *index_page_str;
> char *c_path = NULL;
> struct stat st;
> unsigned int d_cnt, d_i, d_disp = 0;
> int r;
>
> - index_page_str = qs->index_page_str ? qs->index_page_str : "";
> -
> d = opendir(srv->repos_path);
> if (d == NULL) {
> error = got_error_from_errno2("opendir", srv->repos_path);
> @@ -1091,17 +1091,22 @@ render:
> d_disp++;
> t->prev_disp++;
>
> - r = fcgi_printf(c, "<div class='index_wrapper'>\n"
> - "<div class='index_project'>"
> - "<a href='?index_page=%s&path=%s&action=summary'>"
> - " %s "
> - "</a>"
> - "</div>", /* .index_project */
> - index_page_str, repo_dir->name,
> - repo_dir->name);
> + if (fcgi_printf(c, "<div class='index_wrapper'>\n"
> + "<div class='index_project'>") == -1)
> + goto done;
> +
> + r = gotweb_link(c, &(struct gotweb_url){
> + .action = SUMMARY,
> + .index_page = -1,
> + .page = -1,
> + .path = repo_dir->name,
> + }, "%s", repo_dir->name);
> if (r == -1)
> goto done;
>
> + if (fcgi_printf(c, "</div>") == -1) /* .index_project */
> + goto done;
> +
> if (srv->show_repo_description) {
> r = fcgi_printf(c,
> "<div class='index_project_description'>\n"
> @@ -1124,32 +1129,71 @@ render:
> goto done;
> }
>
> - r = fcgi_printf(c, "<div class='navs_wrapper'>"
> - "<div class='navs'>"
> - "<a href='?index_page=%s&path=%s&action=summary'>"
> - "summary"
> - "</a> | "
> - "<a href='?index_page=%s&path=%s&action=briefs'>"
> - "commit briefs"
> - "</a> | "
> - "<a href='?index_page=%s&path=%s&action=commits'>"
> - "commits"
> - "</a> | "
> - "<a href='?index_page=%s&path=%s&action=tags'>"
> - "tags"
> - "</a> | "
> - "<a href='?index_page=%s&path=%s&action=tree'>"
> - "tree"
> - "</a>"
> - "</div>" /* .navs */
> + if (fcgi_printf(c, "<div class='navs_wrapper'>"
> + "<div class='navs'>") == -1)
> + goto done;
> +
> + r = gotweb_link(c, &(struct gotweb_url){
> + .action = SUMMARY,
> + .index_page = -1,
> + .page = -1,
> + .path = repo_dir->name
> + }, "summary");
> + if (r == -1)
> + goto done;
> +
> + if (fcgi_printf(c, " | ") == -1)
> + goto done;
> +
> + r = gotweb_link(c, &(struct gotweb_url){
> + .action = BRIEFS,
> + .index_page = -1,
> + .page = -1,
> + .path = repo_dir->name
> + }, "commit briefs");
> + if (r == -1)
> + goto done;
> +
> + if (fcgi_printf(c, " | ") == -1)
> + goto done;
> +
> + r = gotweb_link(c, &(struct gotweb_url){
> + .action = COMMITS,
> + .index_page = -1,
> + .page = -1,
> + .path = repo_dir->name
> + }, "commits");
> + if (r == -1)
> + goto done;
> +
> + if (fcgi_printf(c, " | ") == -1)
> + goto done;
> +
> + r = gotweb_link(c, &(struct gotweb_url){
> + .action = TAGS,
> + .index_page = -1,
> + .page = -1,
> + .path = repo_dir->name
> + }, "tags");
> + if (r == -1)
> + goto done;
> +
> + if (fcgi_printf(c, " | ") == -1)
> + goto done;
> +
> + r = gotweb_link(c, &(struct gotweb_url){
> + .action = TREE,
> + .index_page = -1,
> + .page = -1,
> + .path = repo_dir->name
> + }, "tree");
> + if (r == -1)
> + goto done;
> +
> + r = fcgi_printf(c, "</div>" /* .navs */
> "<div class='dotted_line'></div>\n"
> - "</div>\n" /* .navs_wrapper */
> - "</div>\n", /* .index_wrapper */
> - index_page_str, repo_dir->name,
> - index_page_str, repo_dir->name,
> - index_page_str, repo_dir->name,
> - index_page_str, repo_dir->name,
> - index_page_str, repo_dir->name);
> + "</div>\n" /* .navs_wrapper */
> + "</div>\n"); /* .index_wrapper */
> if (r == -1)
> goto done;
>
> @@ -1243,13 +1287,10 @@ gotweb_render_briefs(struct request *c)
> struct transport *t = c->t;
> struct querystring *qs = t->qs;
> struct repo_dir *repo_dir = t->repo_dir;
> - const char *index_page_str;
> char *smallerthan, *newline;
> char *age = NULL, *author = NULL, *msg = NULL;
> int r;
>
> - index_page_str = qs->index_page_str ? qs->index_page_str : "";
> -
> r = fcgi_printf(c, "<div id='briefs_title_wrapper'>\n"
> "<div id='briefs_title'>Commit Briefs</div>\n"
> "</div>\n" /* #briefs_title_wrapper */
> @@ -1287,16 +1328,22 @@ gotweb_render_briefs(struct request *c)
>
> 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,
> - author,
> - index_page_str, repo_dir->name, rc->commit_id, qs->headref,
> - msg);
> + "<div class='briefs_log'>",
> + age, author);
> if (r == -1)
> goto done;
>
> + r = gotweb_link(c, &(struct gotweb_url){
> + .action = DIFF,
> + .index_page = -1,
> + .page = -1,
> + .path = repo_dir->name,
> + .commit = rc->commit_id,
> + .headref = qs->headref,
> + }, "%s", msg);
> + if (r == -1)
> + goto done;
> +
> if (rc->refs_str) {
> char *refs;
>
> @@ -1313,18 +1360,38 @@ gotweb_render_briefs(struct request *c)
> goto done;
>
> r = fcgi_printf(c, "<div class='navs_wrapper'>\n"
> - "<div class='navs'>"
> - "<a href='?index_page=%s&path=%s&action=diff&commit=%s"
> - "&headref=%s'>diff</a>"
> - " | "
> - "<a href='?index_page=%s&path=%s&action=tree&commit=%s"
> - "&headref=%s'>tree</a>"
> - "</div>\n" /* .navs */
> + "<div class='navs'>");
> + if (r == -1)
> + goto done;
> +
> + r = gotweb_link(c, &(struct gotweb_url){
> + .action = DIFF,
> + .index_page = -1,
> + .page = -1,
> + .path = repo_dir->name,
> + .commit = rc->commit_id,
> + .headref = qs->headref,
> + }, "diff");
> + if (r == -1)
> + goto done;
> +
> + if (fcgi_printf(c, " | ") == -1)
> + goto done;
> +
> + r = gotweb_link(c, &(struct gotweb_url){
> + .action = TREE,
> + .index_page = -1,
> + .page = -1,
> + .path = repo_dir->name,
> + .commit = rc->commit_id,
> + .headref = qs->headref,
> + }, "tree");
> + if (r == -1)
> + goto done;
> +
> + if (fcgi_printf(c, "</div>\n" /* .navs */
> "</div>\n" /* .navs_wrapper */
> - "<div class='dotted_line'></div>\n",
> - index_page_str, repo_dir->name, rc->commit_id, qs->headref,
> - index_page_str, repo_dir->name, rc->commit_id, qs->headref);
> - if (r == -1)
> + "<div class='dotted_line'></div>\n") == -1)
> goto done;
>
> free(age);
> @@ -1355,14 +1422,10 @@ gotweb_render_commits(struct request *c)
> struct repo_commit *rc = NULL;
> struct server *srv = c->srv;
> struct transport *t = c->t;
> - struct querystring *qs = t->qs;
> struct repo_dir *repo_dir = t->repo_dir;
> - const char *index_page_str;
> char *age = NULL, *author = NULL, *msg = NULL;
> int r;
>
> - index_page_str = qs->index_page_str ? qs->index_page_str : "";
> -
> r = fcgi_printf(c, "<div class='commits_title_wrapper'>\n"
> "<div class='commits_title'>Commits</div>\n"
> "</div>\n" /* .commits_title_wrapper */
> @@ -1404,19 +1467,36 @@ gotweb_render_commits(struct request *c)
> if (r == -1)
> goto done;
>
> - r = fcgi_printf(c, "<div class='navs_wrapper'>\n"
> - "<div class='navs'>"
> - "<a href='?index_page=%s&path=%s&action=diff&commit=%s'>"
> - "diff</a>"
> - " | "
> - "<a href='?index_page=%s&path=%s&action=tree&commit=%s'>"
> - "tree</a>"
> - "</div>\n" /* .navs */
> + if (fcgi_printf(c, "<div class='navs_wrapper'>\n"
> + "<div class='navs'>") == -1)
> + goto done;
> +
> + r = gotweb_link(c, &(struct gotweb_url){
> + .action = DIFF,
> + .index_page = -1,
> + .page = -1,
> + .path = repo_dir->name,
> + .commit = rc->commit_id,
> + }, "diff");
> + if (r == -1)
> + goto done;
> +
> + if (fcgi_printf(c, " | ") == -1)
> + goto done;
> +
> + r = gotweb_link(c, &(struct gotweb_url){
> + .action = TREE,
> + .index_page = -1,
> + .page = -1,
> + .path = repo_dir->name,
> + .commit = rc->commit_id,
> + }, "tree");
> + if (r == -1)
> + goto done;
> +
> + if (fcgi_printf(c, "</div>\n" /* .navs */
> "</div>\n" /* .navs_wrapper */
> - "<div class='dotted_line'></div>\n",
> - index_page_str, repo_dir->name, rc->commit_id,
> - index_page_str, repo_dir->name, rc->commit_id);
> - if (r == -1)
> + "<div class='dotted_line'></div>\n") == -1)
> goto done;
>
> free(age);
> @@ -1449,12 +1529,10 @@ gotweb_render_branches(struct request *c)
> struct transport *t = c->t;
> struct querystring *qs = t->qs;
> struct got_repository *repo = t->repo;
> - const char *index_page_str;
> + char *escaped_refname = NULL;
> char *age = NULL;
> int r;
>
> - index_page_str = qs->index_page_str ? qs->index_page_str : "";
> -
> TAILQ_INIT(&refs);
>
> error = got_ref_list(&refs, repo, "refs/heads",
> @@ -1471,7 +1549,6 @@ gotweb_render_branches(struct request *c)
>
> TAILQ_FOREACH(re, &refs, entry) {
> const char *refname = NULL;
> - char *escaped_refname = NULL;
>
> if (got_ref_is_symbolic(re->ref))
> continue;
> @@ -1498,41 +1575,77 @@ gotweb_render_branches(struct request *c)
> r = fcgi_printf(c, "<div class='branches_wrapper'>\n"
> "<div class='branches_age'>%s</div>\n"
> "<div class='branches_space'> </div>\n"
> - "<div class='branch'>"
> - "<a href='?index_page=%s&path=%s&action=summary&headref=%s'>"
> - "%s</a>"
> - "</div>\n" /* .branch */
> + "<div class='branch'>", age);
> + if (r == -1)
> + goto done;
> +
> + r = gotweb_link(c, &(struct gotweb_url){
> + .action = SUMMARY,
> + .index_page = -1,
> + .page = -1,
> + .path = qs->path,
> + .headref = refname,
> + }, "%s", escaped_refname);
> + if (r == -1)
> + goto done;
> +
> + if (fcgi_printf(c, "</div>\n" /* .branch */
> "<div class='navs_wrapper'>\n"
> - "<div class='navs'>"
> - "<a href='?index_page=%s&path=%s&action=summary&headref=%s'>"
> - "summary</a>"
> - " | "
> - "<a href='?index_page=%s&path=%s&action=briefs&headref=%s'>"
> - "commit briefs</a>"
> - " | "
> - "<a href='?index_page=%s&path=%s&action=commits&headref=%s'>"
> - "commits</a>"
> - "</div>\n" /* .navs */
> - "</div>\n" /* .navs_wrapper */
> + "<div class='navs'>") == -1)
> + goto done;
> +
> + r = gotweb_link(c, &(struct gotweb_url){
> + .action = SUMMARY,
> + .index_page = -1,
> + .page = -1,
> + .path = qs->path,
> + .headref = refname,
> + }, "summary");
> + if (r == -1)
> + goto done;
> +
> + if (fcgi_printf(c, " | ") == -1)
> + goto done;
> +
> + r = gotweb_link(c, &(struct gotweb_url){
> + .action = BRIEFS,
> + .index_page = -1,
> + .page = -1,
> + .path = qs->path,
> + .headref = refname,
> + }, "commit briefs");
> + if (r == -1)
> + goto done;
> +
> + if (fcgi_printf(c, " | ") == -1)
> + goto done;
> +
> + r = gotweb_link(c, &(struct gotweb_url){
> + .action = COMMITS,
> + .index_page = -1,
> + .page = -1,
> + .path = qs->path,
> + .headref = refname,
> + }, "commits");
> + if (r == -1)
> + goto done;
> +
> + r = fcgi_printf(c, "</div>\n" /* .navs */
> + "</div>\n" /* .navs_wrapper */
> "<div class='dotted_line'></div>\n"
> - "</div>\n", /* .branches_wrapper */
> - age ? age : "",
> - index_page_str, qs->path, refname,
> - escaped_refname,
> - index_page_str, qs->path, refname,
> - index_page_str, qs->path, refname,
> - index_page_str, qs->path, refname);
> + "</div>\n"); /* .branches_wrapper */
> + if (r == -1)
> + goto done;
> +
> + free(age);
> + age = NULL;
> free(escaped_refname);
> - if (r == -1)
> - goto done;
> -
> - free(age);
> - age = NULL;
> -
> + escaped_refname = NULL;
> }
> fcgi_printf(c, "</div>\n"); /* #branches_content */
> done:
> free(age);
> + free(escaped_refname);
> got_ref_list_free(&refs);
> return error;
> }
> @@ -1812,12 +1925,9 @@ gotweb_render_tags(struct request *c)
> struct transport *t = c->t;
> struct querystring *qs = t->qs;
> struct repo_dir *repo_dir = t->repo_dir;
> - const char *index_page_str;
> char *age = NULL, *tagname = NULL, *msg = NULL, *newline;
> int r, commit_found = 0;
>
> - index_page_str = qs->index_page_str ? qs->index_page_str : "";
> -
> if (qs->action == BRIEFS) {
> qs->action = TAGS;
> error = got_get_repo_tags(c, D_MAXSLCOMMDISP);
> @@ -1867,32 +1977,66 @@ gotweb_render_tags(struct request *c)
> goto done;
> }
>
> - r = fcgi_printf(c, "<div class='tag_age'>%s</div>\n"
> + if (fcgi_printf(c, "<div class='tag_age'>%s</div>\n"
> "<div class='tag'>%s</div>\n"
> - "<div class='tag_log'>"
> - "<a href='?index_page=%s&path=%s&action=tag&commit=%s'>"
> - "%s</a>"
> - "</div>\n" /* .tag_log */
> + "<div class='tag_log'>", age, tagname) == -1)
> + goto done;
> +
> + r = gotweb_link(c, &(struct gotweb_url){
> + .action = TAG,
> + .index_page = -1,
> + .page = -1,
> + .path = repo_dir->name,
> + .commit = rt->commit_id,
> + }, "%s", msg ? msg : "");
> + if (r == -1)
> + goto done;
> +
> + if (fcgi_printf(c, "</div>\n" /* .tag_log */
> "<div class='navs_wrapper'>\n"
> - "<div class='navs'>"
> - "<a href='?index_page=%s&path=%s&action=tag&commit=%s'>"
> - "tag</a>"
> - " | "
> - "<a href='?index_page=%s&path=%s&action=briefs&commit=%s'>"
> - "commit briefs</a>"
> - " | "
> - "<a href='?index_page=%s&path=%s&action=commits&commit=%s'>"
> - "commits</a>"
> + "<div class='navs'>") == -1)
> + goto done;
> +
> + r = gotweb_link(c, &(struct gotweb_url){
> + .action = TAG,
> + .index_page = -1,
> + .page = -1,
> + .path = repo_dir->name,
> + .commit = rt->commit_id,
> + }, "tag");
> + if (r == -1)
> + goto done;
> +
> + if (fcgi_printf(c, " | ") == -1)
> + goto done;
> +
> + r = gotweb_link(c, &(struct gotweb_url){
> + .action = BRIEFS,
> + .index_page = -1,
> + .page = -1,
> + .path = repo_dir->name,
> + .commit = rt->commit_id,
> + }, "commit briefs");
> + if (r == -1)
> + goto done;
> +
> + if (fcgi_printf(c, " | ") == -1)
> + goto done;
> +
> + r = gotweb_link(c, &(struct gotweb_url){
> + .action = COMMITS,
> + .index_page = -1,
> + .page = -1,
> + .path = repo_dir->name,
> + .commit = rt->commit_id,
> + }, "commits");
> + if (r == -1)
> + goto done;
> +
> + r = fcgi_printf(c,
> "</div>\n" /* .navs */
> "</div>\n" /* .navs_wrapper */
> - "<div class='dotted_line'></div>\n",
> - age,
> - tagname,
> - index_page_str, repo_dir->name, rt->commit_id,
> - 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);
> + "<div class='dotted_line'></div>\n");
> if (r == -1)
> goto done;
>
> @@ -1980,6 +2124,223 @@ done:
> return error;
> }
>
> +static inline int
> +should_urlencode(int c)
> +{
> + if (c <= ' ' || c >= 127)
> + return 1;
> +
> + switch (c) {
> + /* gen-delim */
> + case ':':
> + case '/':
> + case '?':
> + case '#':
> + case '[':
> + case ']':
> + case '@':
> + /* sub-delims */
> + case '!':
> + case '$':
> + case '&':
> + case '\'':
> + case '(':
> + case ')':
> + case '*':
> + case '+':
> + case ',':
> + case ';':
> + case '=':
> + return 1;
> + default:
> + return 0;
> + }
> +}
> +
> +static char *
> +gotweb_urlencode(const char *str)
> +{
> + const char *s;
> + char *escaped;
> + size_t i, len;
> + int a, b;
> +
> + len = 0;
> + for (s = str; *s; ++s) {
> + len++;
> + if (should_urlencode(*s))
> + len += 2;
> + }
> +
> + escaped = calloc(1, len + 1);
> + if (escaped == NULL)
> + return NULL;
> +
> + i = 0;
> + for (s = str; *s; ++s) {
> + if (should_urlencode(*s)) {
> + a = (*s & 0xF0) >> 4;
> + b = (*s & 0x0F);
> +
> + escaped[i++] = '%';
> + escaped[i++] = a <= 9 ? ('0' + a) : ('7' + a);
> + escaped[i++] = b <= 9 ? ('0' + b) : ('7' + b);
> + } else
> + escaped[i++] = *s;
> + }
> +
> + return escaped;
> +}
> +
> +static inline const char *
> +action_name(int action)
> +{
> + switch (action) {
> + case BLAME:
> + return "blame";
> + case BLOB:
> + return "blob";
> + case BRIEFS:
> + return "briefs";
> + case COMMITS:
> + return "commits";
> + case DIFF:
> + return "diff";
> + case ERR:
> + return "err";
> + case INDEX:
> + return "index";
> + case SUMMARY:
> + return "summary";
> + case TAG:
> + return "tag";
> + case TAGS:
> + return "tags";
> + case TREE:
> + return "tree";
> + default:
> + return NULL;
> + }
> +}
> +
> +static int
> +gotweb_print_url(struct request *c, struct gotweb_url *url)
> +{
> + const char *sep = "?", *action;
> + char *tmp;
> + int r;
> +
> + action = action_name(url->action);
> + if (action != NULL) {
> + if (fcgi_printf(c, "?action=%s", action) == -1)
> + return -1;
> + sep = "&";
> + }
> +
> + if (url->commit) {
> + if (fcgi_printf(c, "%scommit=%s", sep, url->commit) == -1)
> + return -1;
> + sep = "&";
> + }
> +
> + if (url->previd) {
> + if (fcgi_printf(c, "%sprevid=%s", sep, url->previd) == -1)
> + return -1;
> + sep = "&";
> + }
> +
> + if (url->prevset) {
> + if (fcgi_printf(c, "%sprevset=%s", sep, url->prevset) == -1)
> + return -1;
> + sep = "&";
> + }
> +
> + if (url->file) {
> + tmp = gotweb_urlencode(url->file);
> + if (tmp == NULL)
> + return -1;
> + r = fcgi_printf(c, "%sfile=%s", sep, tmp);
> + free(tmp);
> + if (r == -1)
> + return -1;
> + sep = "&";
> + }
> +
> + if (url->folder) {
> + tmp = gotweb_urlencode(url->folder);
> + if (tmp == NULL)
> + return -1;
> + r = fcgi_printf(c, "%sfolder=%s", sep, tmp);
> + free(tmp);
> + if (r == -1)
> + return -1;
> + sep = "&";
> + }
> +
> + if (url->headref) {
> + tmp = gotweb_urlencode(url->headref);
> + if (tmp == NULL)
> + return -1;
> + r = fcgi_printf(c, "%sheadref=%s", sep, url->headref);
> + free(tmp);
> + if (r == -1)
> + return -1;
> + sep = "&";
> + }
> +
> + if (url->index_page != -1) {
> + if (fcgi_printf(c, "%sindex_page=%d", sep,
> + url->index_page) == -1)
> + return -1;
> + sep = "&";
> + }
> +
> + if (url->path) {
> + tmp = gotweb_urlencode(url->path);
> + if (tmp == NULL)
> + return -1;
> + r = fcgi_printf(c, "%spath=%s", sep, tmp);
> + free(tmp);
> + if (r == -1)
> + return -1;
> + sep = "&";
> + }
> +
> + if (url->page != -1) {
> + if (fcgi_printf(c, "%spage=%d", sep, url->page) == -1)
> + return -1;
> + sep = "&";
> + }
> +
> + return 0;
> +}
> +
> +int
> +gotweb_link(struct request *c, struct gotweb_url *url, const char *fmt, ...)
> +{
> + va_list ap;
> + int r;
> +
> + if (fcgi_printf(c, "<a href='") == -1)
> + return -1;
> +
> + if (gotweb_print_url(c, url) == -1)
> + return -1;
> +
> + if (fcgi_printf(c, "'>") == -1)
> + return -1;
> +
> + va_start(ap, fmt);
> + r = fcgi_vprintf(c, fmt, ap);
> + va_end(ap);
> + if (r == -1)
> + return -1;
> +
> + if (fcgi_printf(c, "</a>"))
> + return -1;
> + return 0;
> +}
> +
> static struct got_repository *
> find_cached_repo(struct server *srv, const char *path)
> {
> blob - f73ea6b8311442f992dbc1c563f90486ef813c04
> blob + 06f8d01516c1b327e8524a128e571a8cc082ca7e
> --- gotwebd/gotwebd.h
> +++ gotwebd/gotwebd.h
> @@ -341,6 +341,23 @@ struct gotwebd {
> char unix_socket_name[PATH_MAX];
> };
>
> +/*
> + * URL parameter for gotweb_link. NULL values and int set to -1 are
> + * implicitly ignored, and string are properly escaped.
> + */
> +struct gotweb_url {
> + int action;
> + int index_page;
> + int page;
> + const char *commit;
> + const char *previd;
> + const char *prevset;
> + const char *file;
> + const char *folder;
> + const char *headref;
> + const char *path;
> +};
> +
> struct querystring {
> uint8_t action;
> char *commit;
> @@ -411,6 +428,9 @@ const struct got_error
> const struct got_error *gotweb_get_time_str(char **, time_t, int);
> const struct got_error *gotweb_init_transport(struct transport **);
> const struct got_error *gotweb_escape_html(char **, const char *);
> +int gotweb_link(struct request *, struct gotweb_url *, const char *, ...)
> + __attribute__((__format__(printf, 3, 4)))
> + __attribute__((__nonnull__(3)));
> void gotweb_free_repo_commit(struct repo_commit *);
> void gotweb_free_repo_tag(struct repo_tag *);
> void gotweb_process_request(struct request *);
> @@ -426,6 +446,7 @@ void fcgi_timeout(int, short, void *);
> void fcgi_cleanup_request(struct request *);
> void fcgi_create_end_record(struct request *);
> void dump_fcgi_record(const char *, struct fcgi_record_header *);
> +int fcgi_vprintf(struct request *, const char *, va_list);
> int fcgi_printf(struct request *, const char *, ...)
> __attribute__((__format__(printf, 2, 3)))
> __attribute__((__nonnull__(2)));
>
--
Tracey Emery
gotwebd: percent-encode querystrings