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);
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)
> +
> + sprintf(gravatar, "http://www.gravatar.com/avatar/%s?s=";, hex);
Use "//www.gravatar.com/.." instead for HTTPS sites?
> +
> + return gravatar;
> +}
[..]
> --- a/ui-commit.c
> +++ b/ui-commit.c
> @@ -44,6 +44,11 @@ void cgit_print_commit(char *hex, const char *prefix)
> cgit_print_diff_ctrls();
> html("<table summary='commit info' class='commit-info'>\n");
> html("<tr><th>author</th><td>");
> + if (ctx.cfg.enable_gravatar && info->author_gravatar) {
> + html("<img src='");
> + html(info->author_gravatar);
> + html("' width=16 height=16 alt='Gravatar' \\>");
> + }
> html_txt(info->author);
> if (!ctx.cfg.noplainemail) {
> html(" ");
> @@ -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?
> + }
> html_txt(info->committer);
> if (!ctx.cfg.noplainemail) {
> html(" ");
> diff --git a/ui-log.c b/ui-log.c
> index 6f1249b..9974139 100644
> --- a/ui-log.c
> +++ b/ui-log.c
> @@ -168,6 +168,11 @@ static void print_commit(struct commit *commit, struct
> rev_info *revs) sha1_to_hex(commit->object.sha1), ctx.qry.vpath, 0);
> show_commit_decorations(commit);
> html("</td><td>");
> + if (ctx.cfg.enable_gravatar && info->author_gravatar) {
> + html("<img src='");
> + html(info->author_gravatar);
> + html("' width=16 height=16 alt='Gravatar' \\>");
Same here, "/>" instead of "\>".
> + }
> html_txt(info->author);
>
> if (revs->graph) {
> diff --git a/ui-refs.c b/ui-refs.c
> index 0ae0612..8714b2d 100644
> --- a/ui-refs.c
> +++ b/ui-refs.c
> @@ -77,6 +77,11 @@ static int print_branch(struct refinfo *ref)
> if (ref->object->type == OBJ_COMMIT) {
> cgit_commit_link(info->subject, NULL, NULL, name, NULL, NULL,
> 0);
> html("</td><td>");
> + if (ctx.cfg.enable_gravatar && info->author_gravatar) {
> + html("<img src='");
> + html(info->author_gravatar);
> + html("' width=16 height=16 alt='Gravatar' \\>");
Same..
> + }
> html_txt(info->author);
> html("</td><td colspan='2'>");
> cgit_print_age(info->commit->date, -1, NULL);
> @@ -154,6 +159,11 @@ static int print_tag(struct refinfo *ref)
> cgit_object_link(obj);
> html("</td><td>");
> if (info) {
> + if (ctx.cfg.enable_gravatar && info->tagger_gravatar) {
> + html("<img src='");
> + html(info->tagger_gravatar);
> + html("' width=16 height=16 alt='Gravatar' \\>");
Same..
> + }
> if (info->tagger)
> html(info->tagger);
> } else if (ref->object->type == OBJ_COMMIT) {
> diff --git a/ui-tag.c b/ui-tag.c
> index aea7958..ad402da 100644
> --- a/ui-tag.c
> +++ b/ui-tag.c
> @@ -77,6 +77,11 @@ void cgit_print_tag(char *revname)
> }
> if (info->tagger) {
> html("<tr><td>tagged by</td><td>");
> + if (ctx.cfg.enable_gravatar && info->tagger_gravatar) {
> + html("<img src='");
> + html(info->tagger_gravatar);
> + html("' width=16 height=16 alt='Gravatar' \\>");
Yep, also wrong slashes.
> + }
> html_txt(info->tagger);
> if (info->tagger_email && !ctx.cfg.noplainemail) {
> html(" ");
Regards,
Peter
_______________________________________________
CGit mailing list
[email protected]
http://lists.zx2c4.com/mailman/listinfo/cgit
