From: Tracey Emery <tracey@traceyemery.net>
Subject: Re: gotwebd: percent-encode querystrings
To: Omar Polo <op@omarpolo.com>
Cc: gameoftrees@openbsd.org
Date: Tue, 6 Sep 2022 15:38:00 -0600

On Tue, Sep 06, 2022 at 11:30:11PM +0200, Omar Polo wrote:
> 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'.)

ok. thanks!

> 
> 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;
> 

-- 

Tracey Emery