Migrate CurlHandle and derive CurlStream from it. Improves error handling in curl streams and adds covering tests

7 files changed, 166 insertions, 65 deletions

diff --git a/libadhocutil/curlHandle.cpp b/libadhocutil/curlHandle.cpp
new file mode 100644
index 0000000..6315bce
--- /dev/null
+++ b/libadhocutil/curlHandle.cpp
@@ -0,0 +1,71 @@
+#include "curlHandle.h"
+#include <net.h>
+
+static void cleanup() __attribute__((destructor));
+static void cleanup()
+{
+	curl_global_cleanup();
+}
+
+CurlHandle::CurlHandle(const std::string & url) :
+	curl_handle(curl_easy_init()),
+	curl_headers(nullptr),
+	postS(NULL), postE(NULL)
+{
+	curl_easy_setopt(curl_handle, CURLOPT_URL, url.c_str());
+}
+
+CurlHandle::~CurlHandle()
+{
+	if (curl_headers) {
+		curl_slist_free_all(curl_headers);
+	}
+	if (postS) {
+		curl_formfree(postS);
+	}
+	curl_easy_cleanup(curl_handle);
+}
+
+void
+CurlHandle::setopt(CURLoption opt, const void * val)
+{
+	curl_easy_setopt(curl_handle, opt, val);
+}
+
+void
+CurlHandle::appendHeader(const char * header)
+{
+	curl_headers = curl_slist_append(curl_headers, header);
+}
+
+void
+CurlHandle::appendPost(const char * name, const char * value)
+{
+	CURLFORMcode r = curl_formadd(&postS, &postE, CURLFORM_PTRNAME, name, CURLFORM_PTRCONTENTS, value, CURLFORM_END);
+	if (r == 0) {
+		curl_easy_setopt(curl_handle, CURLOPT_HTTPPOST, postS);
+	}
+}
+
+void
+CurlHandle::perform()
+{
+	if (curl_headers) {
+		curl_easy_setopt(curl_handle, CURLOPT_HTTPHEADER, curl_headers);
+	}
+	checkCurlCode(curl_easy_perform(curl_handle));
+}
+
+void
+CurlHandle::checkCurlCode(CURLcode res) const
+{
+	if (res != CURLE_OK) {
+		AdHoc::Net::CurlException ce(res, curl_easy_strerror(res), IceUtil::Optional<Ice::Short>());
+		long http_code = 0;
+		if (curl_easy_getinfo(curl_handle, CURLINFO_RESPONSE_CODE, &http_code) == CURLE_OK) {
+			ce.httpcode = http_code;
+		}
+		throw ce;
+	}
+}
+
diff --git a/libadhocutil/curlHandle.h b/libadhocutil/curlHandle.h
new file mode 100644
index 0000000..75e4b0a
--- /dev/null
+++ b/libadhocutil/curlHandle.h
@@ -0,0 +1,32 @@
+#ifndef ADHOCUTIL_CURLHANDLE_H
+#define ADHOCUTIL_CURLHANDLE_H
+
+#include <curl/curl.h>
+#include "intrusivePtrBase.h"
+#include "visibility.h"
+
+class DLL_PUBLIC CurlHandle : public virtual IntrusivePtrBase {
+	public:
+		CurlHandle(const std::string & url);
+		virtual ~CurlHandle();
+
+		CurlHandle(const CurlHandle &) = delete;
+		void operator=(const CurlHandle &) = delete;
+
+		void setopt(CURLoption opt, const void * val);
+		void getinto(CURLoption opt, void * & val);
+		void appendHeader(const char *);
+		void appendPost(const char *, const char *);
+		void perform();
+
+	protected:
+		void checkCurlCode(CURLcode res) const;
+
+		CURL * curl_handle;
+		curl_slist * curl_headers;
+		curl_httppost * postS, * postE;
+};
+typedef boost::intrusive_ptr<CurlHandle> CurlHandlePtr;
+
+#endif
+
diff --git a/libadhocutil/curlStream.cpp b/libadhocutil/curlStream.cpp
index 850dea8..e31c99c 100644
--- a/libadhocutil/curlStream.cpp
+++ b/libadhocutil/curlStream.cpp
@@ -1,40 +1,21 @@
 #include "curlStream.h"
-#include <net.h>
 
 CurlStreamSource::CurlStreamSource(const std::string & url) :
-	curl_handle(curl_easy_init()),
-	curl_headers(nullptr),
+	CurlHandle(url),
 	buflen(0),
 	buf(nullptr),
 	res(CURLE_OK)
 {
-	curl_easy_setopt(curl_handle, CURLOPT_URL, url.c_str());
 	curl_easy_setopt(curl_handle, CURLOPT_WRITEFUNCTION, &CurlStreamSource::recvWrapper);
 	curl_easy_setopt(curl_handle, CURLOPT_WRITEDATA, this);
 }
 
-CurlStreamSource::~CurlStreamSource()
-{
-	if (curl_headers) {
-		curl_slist_free_all(curl_headers);
-	}
-	if (res != CURLE_OK) {
-		AdHoc::Net::CurlException ce(res, curl_easy_strerror(res), IceUtil::Optional<Ice::Short>());
-		long http_code = 0;
-		if (curl_easy_getinfo(curl_handle, CURLINFO_RESPONSE_CODE, &http_code) == CURLE_OK) {
-			ce.httpcode = http_code;
-		}
-		curl_easy_cleanup(curl_handle);
-		throw ce;
-	}
-	curl_easy_cleanup(curl_handle);
-}
-
 std::streamsize
 CurlStreamSource::read(char * target, std::streamsize targetSize)
 {
 	if (!buflen) {
 		SwapContext();
+		checkCurlCode(res);
 	}
 	size_t bytes = std::min<size_t>(buflen, targetSize);
 	memcpy(target, buf, bytes);
@@ -44,18 +25,6 @@ CurlStreamSource::read(char * target, std::streamsize targetSize)
 }
 
 void
-CurlStreamSource::setopt(CURLoption opt, const void * & val)
-{
-	curl_easy_setopt(curl_handle, opt, val);
-}
-
-void
-CurlStreamSource::appendHeader(const char * header)
-{
-	curl_headers = curl_slist_append(curl_headers, header);
-}
-
-void
 CurlStreamSource::Callback()
 {
 	if (curl_headers) {
diff --git a/libadhocutil/curlStream.h b/libadhocutil/curlStream.h
index 459a472..cb003bb 100644
--- a/libadhocutil/curlStream.h
+++ b/libadhocutil/curlStream.h
@@ -6,28 +6,20 @@
 #include <string>
 #include <curl/curl.h>
 #include "visibility.h"
+#include "curlHandle.h"
 
-class DLL_PUBLIC CurlStreamSource : public boost::iostreams::source, RuntimeContext {
+class DLL_PUBLIC CurlStreamSource : public boost::iostreams::source, public CurlHandle, RuntimeContext {
 	public:
 		CurlStreamSource(const std::string & url);
-		~CurlStreamSource();
-
-		CurlStreamSource(const CurlStreamSource &) = delete;
-		void operator=(const CurlStreamSource &) = delete;
 
 		std::streamsize read(char * target, std::streamsize targetSize);
 
-		void setopt(CURLoption opt, const void * & val);
-		void appendHeader(const char * header);
-
 	private:
 		DLL_PRIVATE void Callback() override;
 
 		DLL_PRIVATE static size_t recvWrapper(void * data, size_t sz, size_t nm, void * css);
 		DLL_PRIVATE size_t recv(void * data, size_t datalen);
 
-		CURL * curl_handle;
-		struct curl_slist * curl_headers;
 		size_t buflen;
 		char * buf;
 		CURLcode res;
diff --git a/libadhocutil/unittests/Jamfile.jam b/libadhocutil/unittests/Jamfile.jam
index 724db34..7b2720e 100644
--- a/libadhocutil/unittests/Jamfile.jam
+++ b/libadhocutil/unittests/Jamfile.jam
@@ -19,7 +19,7 @@ run
 	;
 
 run
-	testCurlStream.cpp
+	testCurl.cpp
 	: : :
 	<define>ROOT=\"$(me)\"
 	<define>BOOST_TEST_DYN_LINK
@@ -27,8 +27,9 @@ run
 	<library>boost_utf
 	<library>boost_filesystem
 	<library>boost_system
+	<implicit-dependency>..//adhocutil
 	:
-	testCurlStream
+	testCurl
 	;
 
 run
diff --git a/libadhocutil/unittests/testCurl.cpp b/libadhocutil/unittests/testCurl.cpp
new file mode 100644
index 0000000..1c0ed7f
--- /dev/null
+++ b/libadhocutil/unittests/testCurl.cpp
@@ -0,0 +1,56 @@
+#define BOOST_TEST_MODULE Curl
+#include <boost/test/unit_test.hpp>
+
+#include "curlHandle.h"
+#include "curlStream.h"
+#include "definedDirs.h"
+#include "net.h"
+
+size_t discard(void *, size_t sz, size_t nm, void *)
+{
+	return sz * nm;
+}
+
+BOOST_AUTO_TEST_CASE( fetch_file )
+{
+	auto url = "file://" + RootDir.string() + "/testCurl.cpp";
+	CurlHandle ch(url);
+	ch.setopt(CURLOPT_WRITEFUNCTION, (void*)&discard);
+	ch.perform();
+}
+
+BOOST_AUTO_TEST_CASE( fetch_missing )
+{
+	auto url = "file://" + RootDir.string() + "/nothere";
+	CurlHandle ch(url);
+	BOOST_REQUIRE_THROW(ch.perform(), AdHoc::Net::CurlException);
+}
+
+BOOST_AUTO_TEST_CASE( fetch_file_stream )
+{
+	auto url = "file://" + RootDir.string() + "/testCurl.cpp";
+	CurlStreamSource css(url);
+	boost::iostreams::stream<css_ref> curlstrm(boost::ref(css));
+	std::string tok;
+	curlstrm >> tok;
+	BOOST_REQUIRE_EQUAL("#define", tok);
+	curlstrm >> tok;
+	BOOST_REQUIRE_EQUAL("BOOST_TEST_MODULE", tok);
+	curlstrm >> tok;
+	BOOST_REQUIRE_EQUAL("Curl", tok);
+	while (!curlstrm.eof()) {
+		curlstrm >> tok;
+	}
+}
+
+BOOST_AUTO_TEST_CASE( fetch_missing_stream )
+{
+	auto url = "file://" + RootDir.string() + "/nothere";
+	BOOST_REQUIRE_THROW({
+		CurlStreamSource css(url);
+		boost::iostreams::stream<css_ref> curlstrm(boost::ref(css));
+		std::string tok;
+		curlstrm >> tok;
+	}, AdHoc::Net::CurlException);
+}
+
diff --git a/libadhocutil/unittests/testCurlStream.cpp b/libadhocutil/unittests/testCurlStream.cpp
deleted file mode 100644
index 48e3c27..0000000
--- a/libadhocutil/unittests/testCurlStream.cpp
+++ /dev/null
@@ -1,20 +0,0 @@
-#define BOOST_TEST_MODULE CurlStream
-#include <boost/test/unit_test.hpp>
-
-#include "curlStream.h"
-#include "definedDirs.h"
-
-BOOST_AUTO_TEST_CASE( fetch_file )
-{
-	auto url = "file://" + RootDir.string() + "/testCurlStream.cpp";
-	CurlStreamSource css(url);
-	boost::iostreams::stream<css_ref> curlstrm(boost::ref(css));
-	std::string tok;
-	curlstrm >> tok;
-	BOOST_REQUIRE_EQUAL("#define", tok);
-	curlstrm >> tok;
-	BOOST_REQUIRE_EQUAL("BOOST_TEST_MODULE", tok);
-	curlstrm >> tok;
-	BOOST_REQUIRE_EQUAL("CurlStream", tok);
-}
-