cURL / Mailing Lists / curl-library / Single Mail

curl-library

Re: Peer review! SecureTransport (native SSL on iOS/OS X) patch

From: Daniel Stenberg <daniel_at_haxx.se>
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.html
Received on 2012-06-25