Created attachment 217277 [details] proposed patch Storage and check the whole certificate chain, not just the root certificate.
Comment on attachment 217277 [details] proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=217277&action=review Looks good overall. I have some concern about the BIO_get_mem_data call (though I realize this wasn't code you added). Can you review and let me know what you think? > Source/WebCore/platform/network/curl/SSLHandle.cpp:146 > + unsigned char *certificateData; unsigned char* certificateData; > Source/WebCore/platform/network/curl/SSLHandle.cpp:147 > + long len = BIO_get_mem_data(bio, &certificateData); The BIO_get_mem_data documentation is pretty weak. Does it ever return a negative value? If not, why is the return value signed? If it is negative, then the following code will do bad things.
(In reply to comment #1) > (From update of attachment 217277 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=217277&action=review > Thanks for the detailed review. > Looks good overall. I have some concern about the BIO_get_mem_data call (though I realize this wasn't code you added). Can you review and let me know what you think? > BIO_get_mem_data() sets pp to a pointer to the start of the memory BIOs data and returns the total amount of data available. length = BIO_get_mem_data(bio, &certificateData); // here - certificateData is a pointer to encoded data, length - length of data. > > Source/WebCore/platform/network/curl/SSLHandle.cpp:146 > > + unsigned char *certificateData; > > unsigned char* certificateData; > I changed this. > > Source/WebCore/platform/network/curl/SSLHandle.cpp:147 > > + long len = BIO_get_mem_data(bio, &certificateData); > > The BIO_get_mem_data documentation is pretty weak. Does it ever return a negative value? If not, why is the return value signed? If it is negative, then the following code will do bad things. Yes, I made the change what is necessary to avoid unexpected behaviors in Bug119436 which this bug depends on.
Created attachment 217534 [details] proposed patch
Created attachment 217537 [details] proposed patch Add missing ChangeLog
Comment on attachment 217537 [details] proposed patch r=me.
Comment on attachment 217537 [details] proposed patch Clearing flags on attachment: 217537 Committed r159632: <http://trac.webkit.org/changeset/159632>
All reviewed patches have been landed. Closing bug.