Download raw body.
gotwebd: percent-encode querystrings
On Tue, Sep 06, 2022 at 01:17:21PM -0600, Tracey Emery wrote:
> 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.
No sorry, looks like something else broke that. But no time to test
right now. Look at briefs. Clicking next just now builds on the page
instead of the original, correct action.
>
> >
> > 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
>
--
Tracey Emery
gotwebd: percent-encode querystrings