From: Omar Polo <op@omarpolo.com>
Subject: Re: gotwebd: percent-encode querystrings
To: Tracey Emery <tracey@traceyemery.net>
Cc: gameoftrees@openbsd.org
Date: Tue, 06 Sep 2022 23:30:11 +0200

On 2022/09/06 13:21:10 -0600, Tracey Emery <tracey@traceyemery.net> wrote:
> 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.

yeah, it seems that i've broken the briefs navigation in
4a962942057bae9fbd47916e75d554fb52e0ec37 D:

got_get_repo_commits iterates the commit and build the queue.  My
memleak fix moved the TAILQ_INSERT_TAIL *before* hte logic to exit the
loop and so it broke the pagination.

Fixing it is not completely straightforward because the old code
relied on leaking repo_commit at every iteration to compute the
previous button.

So, other than restoring the TAILQ_INSERT_TAIL call positioning diff
below moves the check for the previous button after
got_get_repo_commit.  (note how the check was changed from `limit - 1'
to `limit'.)

diff /home/op/w/got
commit - 9e03e308683be9a2f5e4ff74c3b135c1024bd876
path + /home/op/w/got
blob - 5211e085ce0097066a9ec6cb94c843ffc952f29f
file + gotwebd/got_operations.c
--- gotwebd/got_operations.c
+++ gotwebd/got_operations.c
@@ -436,16 +436,6 @@ got_get_repo_commits(struct request *c, int limit)
 	for (;;) {
 		struct got_object_id *next_id;
 
-		if (limit_chk == ((limit * qs->page) - (limit - 1)) &&
-		    commit_found == 0 && repo_commit &&
-		    repo_commit->commit_id != NULL) {
-			t->prev_id = strdup(repo_commit->commit_id);
-			if (t->prev_id == NULL) {
-				error = got_error_from_errno("strdup");
-				goto done;
-			}
-		}
-
 		error = got_commit_graph_iter_next(&next_id, graph, repo, NULL,
 		    NULL);
 		if (error) {
@@ -467,13 +457,21 @@ got_get_repo_commits(struct request *c, int limit)
 		if (error)
 			goto done;
 
-		TAILQ_INSERT_TAIL(&t->repo_commits, repo_commit, entry);
-
 		error = got_get_repo_commit(c, repo_commit, commit,
 		    &refs, next_id);
 		if (error)
 			goto done;
 
+		if (limit_chk == ((limit * qs->page) - limit) &&
+		    commit_found == 0 && repo_commit->commit_id != NULL) {
+			t->prev_id = strdup(repo_commit->commit_id);
+			if (t->prev_id == NULL) {
+				error = got_error_from_errno("strdup");
+				gotweb_free_repo_commit(repo_commit);
+				goto done;
+			}
+		}
+
 		if (qs->commit != NULL && commit_found == 0 && limit != 1) {
 			if (strcmp(qs->commit, repo_commit->commit_id) == 0)
 				commit_found = 1;
@@ -481,11 +479,14 @@ got_get_repo_commits(struct request *c, int limit)
 			    qs->page == 0)
 				commit_found = 1;
 			else {
+				gotweb_free_repo_commit(repo_commit);
 				limit_chk++;
 				continue;
 			}
 		}
 
+		TAILQ_INSERT_TAIL(&t->repo_commits, repo_commit, entry);
+
 		if (limit == 1 && chk_multi == 0 &&
 		    srv->max_commits_display != 1)
 			commit_found = 1;