curl-library
Re: Peer review! SecureTransport (native SSL on iOS/OS X) patch
Date: Mon, 25 Jun 2012 12:07:33 +0200 (CEST)
On Sun, 24 Jun 2012, Nick Zitzmann wrote:
> I couldn't let Windows users have all the fun with the next major release of
> Curl, so yesterday I added preliminary support for SecureTransport.
Lovely!
Some initial comments:
o I had to remove the C++/c99 comment from lib/securetransport.h to be able
to build since I use -Werror and enforce c89...
o please use memset() instead of bzero() for consistency
o regarding the ioErr = -36, doesn't that value have a name in the header that
you can include in the comment to help a reader understand it? Also, I would
prefer a #define to a static const...
> 1. All of the tests that pass with OpenSSL enabled pass with SecureTransport
> enabled, except for test 405. How do I debug a specific unit test so I can
> figure out what's going on here? I'd prefer using GDB, but if that won't
> work, I'm wondering if there's a way to see the verbose output.
1. ./configure --enable-debug --disable-shared
... and when you do this, you'll notice that lib/checksrc.pl will
automatically run and check your source code formatting for some of the most
common mistakes and you'll find out that it found _98_ warnings in your
patch... most of them seems to be trailing whitespace. It might be a good idea
to fix them before your follow patch post! =)
2. make && make -C tests
3. cd tests
4. ./runtests.pl -g 405
This last command will fire up the environment correctly and use gdb to invoke
the correct command. You'd then start with setting your break-points or
whatever and then enter 'run' and watch the test case run.
> 2. Would anyone mind if I added a new selector to the curl_easy_getinfo()
> function that returns an OS-specific data structure? I ask because, if the
> trust fails, it's customary on Mac OS X for the app to present a certificate
> error window to the user showing the problem and asking if the user wants to
> proceed or cancel the connection . To do that, Curl needs to be able to
> share the underlying trust data structure (which is called "SecTrustRef")
> with the rest of the app. I thought about using CURLINFO_CERTINFO, which is
> the OpenSSL equivalent, but since the return type is totally different,
> that'll break every app that expected a curl_certinfo list in return.
I might be OK with that if there truly is no other way. But is this really an
OSX specific thing? A failed certificate check happens on all platforms for
example when self-signed certs are used or similar.
-- / daniel.haxx.se ------------------------------------------------------------------- List admin: http://cool.haxx.se/list/listinfo/curl-library Etiquette: http://curl.haxx.se/mail/etiquette.htmlReceived on 2012-06-25