[PATCH 1/1] enable cgit to show gravatar for author, committer and tagger

[Christian Hesse]

Peter Wu <lekensteyn at gmail.com> on Wed, 2013/11/27 17:46:
> Hi,
> 
> See below for review.
> 
> On Wednesday 27 November 2013 16:17:17 Christian Hesse wrote:
> > ---
> >  cgit.c       |  3 +++
> >  cgit.h       |  5 +++++
> >  cgitrc.5.txt |  4 ++++
> >  parsing.c    | 47 +++++++++++++++++++++++++++++++++++++++++++++++
> >  shared.c     |  6 ++++++
> >  ui-commit.c  | 10 ++++++++++
> >  ui-log.c     |  5 +++++
> >  ui-refs.c    | 10 ++++++++++
> >  ui-tag.c     |  5 +++++
> >  9 files changed, 95 insertions(+)
> 
> [..]
> 
> > @@ -258,3 +273,35 @@ struct taginfo *cgit_parse_tag(struct tag *tag)
> >  	free(data);
> >  	return ret;
> >  }
> > +
> > +char * cgit_get_gravatar(const char * email) {
> > +    	int n, length;
> > +	MD5_CTX c;
> > +	unsigned char digest[16];
> > +	char hex[33];
> > +	char * gravatar = malloc(67);
> > +
> > +	/* skip brackets! */
> > +	email++;
> > +	length = strlen(email) - 1;
> 
> > +
> > +	MD5_Init(&c);
> > +
> > +	while (length > 0) {
> > +		if (length > 512)
> > +			MD5_Update(&c, email, 512);
> > +		else
> > +			MD5_Update(&c, email, length);
> Seems like an error from Stackoverflow[1]? According to the manual page,
> the length is in bytes, not bits.
> 
>  [1]: http://stackoverflow.com/a/8389763/427545
> 
> > +		length -= 512;
> > +		email += 512;
> > +	}
> > +
> > +	MD5_Final(digest, &c);
> 
> Since you know the full string onbeforehand, you should also be able to use 
> MD5() directly. Something like (openssl):
> 
>     MD5(email, length, digest);

Updated it to use MD5(...).

Though the licensing issue remain...

> As you mentioned, email addresses should be converted to lower case first 
> according to http://en.gravatar.com/site/implement/hash/. The below
> str_to_hex was written before I knew that.
> 
> > +
> > +	for (n = 0; n < 16; ++n)
> > +		snprintf(&(hex[n*2]), 16*2, "%02x", (unsigned
> > int)digest[n]);
> 
> This 16*2 is bogus, if you are at the 30th position, you only have two
> bytes left. What about adding a new md5_to_hex() function, similar to
> sha1_to_hex() in git? Here is a generic approach:
> 
> /* gets a string representing the binary data. dst_str must have a
>  * size of at least src_len * 2 + 1 bytes. */
>     void str_to_hex(char *dst_str, const unsigned char *src_bytes,
>                     size_t src_len) {
>         const char *hex = "0123456789abcdef";
>         int n;
> 
>         for (n = 0; n < src_len; ++n) {
>             unsigned char val = *src_bytes++;
>             *dst_str++ = hex[val >> 4];
>             *dst_str++ = hex[val & 0xf];
>         }
> 
>         *dst_str = 0;
>     }
> 
>     str_to_hex(hex, digest, 16);
> 
> The emailaddress limitation could probably be resolved here by repeatedly 
> calling:
> 
>     MD5_Update(&c, tolower(email[n]), 1)

I updated my code. It duplicates the string now, makes it lower case and uses
your function to calculate hex.

> > +
> > +	sprintf(gravatar, "http://www.gravatar.com/avatar/%s?s=", hex);
> 
> Use "//www.gravatar.com/.." instead for HTTPS sites?

Did not know this works. Great!

> > +
> > +	return gravatar;
> > +}
> 
> [..]
>
> > @@ -53,6 +58,11 @@ void cgit_print_commit(char *hex, const char *prefix)
> >  	cgit_print_date(info->author_date, FMT_LONGDATE,
> > ctx.cfg.local_time); html("</td></tr>\n");
> >  	html("<tr><th>committer</th><td>");
> > +	if (ctx.cfg.enable_gravatar && info->committer_gravatar) {
> > +		html("<img src='");
> > +		html(info->committer_gravatar);
> > +		html("' width=16 height=16 alt='Gravatar' \\>");
> 
> SHouldn't that \\ become a / for XHTML? 

I was sitting in my car on the way home when I thought about why I had to
escape this...

Of course you are right.

[...]

Thanks for your review! I will reply with an updated patch later.
-- 
main(a){char*c=/*    Schoene Gruesse                         */"B?IJj;MEH"
"CX:;",b;for(a/*    Chris           get my mail address:    */=0;b=c[a++];)
putchar(b-1/(/*               gcc -o sig sig.c && ./sig    */b/42*2-3)*42);}
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 490 bytes
Desc: not available
URL: <http://lists.zx2c4.com/pipermail/cgit/attachments/20131127/5ebac967/attachment.asc>