Bug 3473 - base64 regression with old glib2 (before 2.26.0)
Summary: base64 regression with old glib2 (before 2.26.0)
Status: RESOLVED FIXED
Alias: None
Product: Claws Mail
Classification: Unclassified
Component: Other (show other bugs)
Version: 3.13.0
Hardware: PC Linux
: P3 normal
Assignee: users
URL:
Depends on:
Blocks:
 
Reported: 2015-07-21 17:21 CEST by Jean Diraison
Modified: 2015-08-16 01:42 CEST (History)
2 users (show)

See Also:


Attachments
g_base64_decode() work around patch for olds glib2 (1.53 KB, patch)
2015-07-26 17:53 CEST, Jean Diraison
no flags Details | Diff
g_base64_decode() wrapper for olds glib2 (1.44 KB, patch)
2015-07-29 17:37 CEST, Jean Diraison
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jean Diraison 2015-07-21 17:21:08 CEST
The g_base64_decode() function fails with a short input (when length is 0 or 1) with old glib2, due to some checkings :

	g_return_val_if_fail (input_length > 1, NULL);

http://sourcecodebrowser.com/glib2.0/2.25.9/gbase64_8c.html#a63c6d3fe0225174fef9b924880d06034
http://sourcecodebrowser.com/glib2.0/2.26.0/gbase64_8c.html#a63c6d3fe0225174fef9b924880d06034

This behavior generate claws-mail crashed when trying to decode empty passwords (like "password=!" entry) in the accountrc file.
Comment 1 MD 2015-07-21 21:07:07 CEST
We have problems along the same line.

We had no problems compiling and using Claws 3.11 in RHEL 6.1.

But were unable to compile Claws 3.12 in RHEL 6.1, apparently due to changes in glib2.  It will compile fine in 6.6, but not 6.1 (or presumably anything older, such as RHEL 5).  "Ticho" on IRC created a patch for 3.12 that allowed it to compile in 6.1 using the older glib2.  But the binary it creates crashes after the account setup with this:

(claws-mail:26555): GLib-CRITICAL **: g_base64_decode: assertion `input_length > 1' failed
Segmentation fault (core dumped)

The patch was added to mainline, so anyone downloading it since yesterday will likely have the same issue using it on older systems until something changes.
Comment 2 Jean Diraison 2015-07-26 17:53:24 CEST
Created attachment 1549 [details]
g_base64_decode() work around patch for olds glib2

This patch transparently redefines g_base64_decode() function with old glib2 (pre 2.26.0). It is for git version, cause it has to be applied over "[PATCH] Fix building on GLib older than 2.25." from Andrej Kacian aab231b16fd07a9196fe882205658d33c78752d2
Comment 3 Andrej Kacian 2015-07-28 21:06:05 CEST
Jean, have you tested that your patch indeed works with an older glib? I'm asking just to be sure, because I have no way to test it at the moment.
Comment 4 Jean Diraison 2015-07-28 22:47:59 CEST
Hello Andrej. It works without any problem until now on my system with an old 2.20.5 version of the glib2 library.
Comment 5 Ricardo Mones 2015-07-29 10:36:41 CEST
I think the current NULL checks for parameters¹ should be added to the patch.

And, oh, also the attribution to the origin of the code :-)

¹ https://github.com/GNOME/glib/blob/master/glib/gbase64.c#L387
Comment 6 Jean Diraison 2015-07-29 17:37:43 CEST
Created attachment 1550 [details]
g_base64_decode() wrapper for olds glib2

Here it is a new patch without any glib2 source code copy, to prevent license problem.

It includes g_base64_decode_wrapper() a wrapper function of g_base64_decode(). This wrapper manages errors by returning a valid empty string pointer like g_base64_decode() from recent glib2.
Comment 7 Ricardo Mones 2015-08-01 20:15:10 CEST
(In reply to comment #6)
> Created attachment 1550 [details]
> g_base64_decode() wrapper for olds glib2
> 
> Here it is a new patch without any glib2 source code copy, to prevent
> license problem.

There's no license problem for copying glib sources, it's LGPLv2+¹ and it's already been done before in Claws Mail source.

I was only pointing out that if you do copy it, just say who's the author in the copy. Nothing more, nothing less ;-)

¹ http://blog.cberger.net/2008/06/12/lgplv2-is-compatible-with-lgplv3-and-later/
Comment 8 users 2015-08-13 20:27:02 CEST
Changes related to this bug have been committed.
Please check latest Git and update the bug accordingly.
You can also get the patch from:
http://git.claws-mail.org/

++ ChangeLog	2015-08-13 20:27:02.627467533 +0200
http://git.claws-mail.org/?p=claws.git;a=commitdiff;h=479d3cd2ebe57b8fa8454b79a867fb21319ea88c
Merge: ed28285 bf373b8
Author: Colin Leroy <colin@colino.net>
Date:   Thu Aug 13 20:27:02 2015 +0200

    Merge branch 'master' of file:///home/git/claws

http://git.claws-mail.org/?p=claws.git;a=commitdiff;h=bf373b8226f1b3b335619a75de80041ee5ad9761
Author: Andrej Kacian <ticho@claws-mail.org>
Date:   Thu Aug 13 20:24:48 2015 +0200

    Provide an up to date version of g_base64_decode() for older GLib.
    
    Patch by Jean Diraison, closes bug #3473.
Comment 9 Andrej Kacian 2015-08-13 20:40:49 CEST
Thanks Jean, I have added the first patch.

Ricardo, let me know if a more explicit attribution for the included Glib function is needed, and what form should it have in our code.
Comment 10 Ricardo Mones 2015-08-16 01:42:08 CEST
(In reply to comment #9)
> Thanks Jean, I have added the first patch.
> 
> Ricardo, let me know if a more explicit attribution for the included Glib
> function is needed, and what form should it have in our code.

I guess it could be the same as other copies, see header of utils.c

http://git.claws-mail.org/?p=claws.git;a=blob;f=src/common/utils.c;h=64ca25a8#l18

regards,