GNOME Bugzilla – Bug 437331
Read NSS certificate files more reliably
Last modified: 2007-10-07 17:22:08 UTC
I rewrote the part of Camel that reads NSS certificates in an attempt to fix a downstream bug about Evolution crashing at startup [1]. camel_certdb_nss_cert_get() currently calls read() to read the certificate file and tries to handle all the corner cases that come with using system I/O calls directly (e.g. interrupts, error conditions, short reads). The reporter of the downstream bug is seeing warning messages emanating from this function before Evolution crashes, so my assumption is that the read logic is not properly handling some corner case. I rewrote the read logic to just call g_file_get_contents(), which handles all these corner cases for you. [1] http://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=215634
Created attachment 87927 [details] [review] Proposed patch
Philip, I thought you might be interested in this.
hmmm, path is getting free'd and then used later (original code). also I'm not sure I see any problem with the previous read loop, tho it probably should have reused camel-file-utils.c's i/o wrappers.
Agreed, it should be calling some higher-level function than read(), be it in Camel or GLib. The reporter is seeing the following warnings that seem to indicate the file is not being read correctly. It's hard to say what's causing the read() failure since the original code doesn't output strerror(errno). (evolution:3032): camel-WARNING **: rawcert != derCer (evolution:3032): camel-WARNING **: cert size read truncated X\xf7r\xb4: 0 != 827
As per comment #3 updating patch status.
I think either Camel's I/O wrappers or g_file_get_contents() would work fine. Camel's I/O wrappers seems like a bit of an overkill to me; this operation doesn't need to be cancellable.
comment #2: Indeed. I will take a look. Thanks.
Changing the patch status back to "none", as I feel it's ready for review as is.
Ah! I misread it. Sorry for the confusion for the patch state. Varadhan: Can you review this for 2.11.4?
I'm going to confirm this bug since the downstream reporter claims the patch fixed his crash (see [1] and subsequent comments). [1] http://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=215634#c21
Patch looks good. Please commit to trunk with a ChangeLog.
Committed to trunk (revision 7982).
Committed this patch to Tinymail's camel-lite. http://tinymail.org/trac/tinymail/changeset/2824