From: Omar Polo <op@omarpolo.com>
Subject: Re: gotwebd: some html fixes
To: Omar Polo <op@omarpolo.com>
Cc: gameoftrees@openbsd.org
Date: Fri, 19 Aug 2022 19:36:30 +0200

On 2022/08/19 19:30:59 +0200, Omar Polo <op@omarpolo.com> wrote:
> 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?

oh, i forgot to mention one thing:

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

i've included it by mistake but since it's here... it's a fix for a
possible NULL owner, reported by tracey@ on irc.