libcurl integration and HTTP support enhanced

Issue was: the asynch interface for HTTP access
wasn't entirely compatible between Curl and Qt
implementations.

Plus, the progress integration was messed up
because of the deleteLater scheme (progress was
visible even after the connection ended).

The deleteLater scheme was replaced by an
explicit close which does not require the
connection to be deleted immediately in the
receivers.

src/lay/lay/laySaltManagerDialog.cc

@@ -687,7 +687,10 @@ void
 SaltManagerDialog::salt_mine_download_finished ()
 {
   QApplication::restoreOverrideCursor ();
-  m_salt_mine_reader.reset (0);
+  if (m_salt_mine_reader.get ()) {
+    //  NOTE: don't delete the reader in the slot it triggered
+    m_salt_mine_reader->close ();
+  }
 }
 
 void
@@ -951,7 +954,10 @@ SaltManagerDialog::get_remote_grain_info (lay::SaltGrain *g, SaltGrainDetailsTex
   }
 
   m_downloaded_grain.reset (0);
-  m_downloaded_grain_reader.reset (0);
+  if (m_downloaded_grain_reader.get ()) {
+    //  NOTE: don't delete the reader in the slot it triggered
+    m_downloaded_grain_reader->close ();
+  }
   mp_downloaded_target = details;
   m_salt_mine_grain.reset (new lay::SaltGrain (*g));
 
@@ -1022,7 +1028,10 @@ SaltManagerDialog::data_ready ()
     mp_downloaded_target->set_grain (m_downloaded_grain.get ());
 
     m_downloaded_grain.reset (0);
-    m_downloaded_grain_reader.reset (0);
+    if (m_downloaded_grain_reader.get ()) {
+      //  NOTE: don't delete the reader in the slot it triggered
+      m_downloaded_grain_reader->close ();
+    }
     m_salt_mine_grain.reset (0);
 
   } catch (tl::Exception &ex) {
@@ -1049,7 +1058,10 @@ SaltManagerDialog::show_error (tl::Exception &ex)
   mp_downloaded_target->setHtml (html);
 
   m_downloaded_grain.reset (0);
-  m_downloaded_grain_reader.reset (0);
+  if (m_downloaded_grain_reader.get ()) {
+    //  NOTE: don't delete the reader in the slot it triggered
+    m_downloaded_grain_reader->close();
+  }
   m_salt_mine_grain.reset (0);
 }
 

src/tl/tl/tlHttpStreamCurl.cc

@@ -406,6 +406,11 @@ public:
    */
   void send ();
 
+  /**
+   *  @brief Closes this connection
+   */
+  void close ();
+
   /**
    *  @brief Gets the HTTP status after the finished_event has been triggered
    */
@@ -568,6 +573,19 @@ CurlConnection::CurlConnection (const CurlConnection &other)
   init ();
 }
 
+void CurlConnection::close ()
+{
+  CurlNetworkManager::instance ()->release_connection (this);
+  curl_slist_free_all (mp_headers);
+
+  mp_handle = 0;
+  m_http_status = 0;
+  m_finished = false;
+  m_status = 0;
+  mp_headers = 0;
+  m_authenticated = 0;
+}
+
 void CurlConnection::init ()
 {
   m_http_status = 0;
@@ -584,8 +602,10 @@ CurlConnection::~CurlConnection ()
 #if defined(DEBUG_CURL)
   std::cerr << "~CurlConnection(" << (void *)mp_handle << ")" << std::endl;
 #endif
-  CurlNetworkManager::instance ()->release_connection (this);
-  curl_slist_free_all (mp_headers);
+  if (mp_handle) {
+    CurlNetworkManager::instance ()->release_connection (this);
+    curl_slist_free_all (mp_headers);
+  }
 }
 
 void CurlConnection::set_url (const char *url)
@@ -648,6 +668,8 @@ size_t CurlConnection::fetch_data (char *buffer, size_t nbytes)
 
 void CurlConnection::send ()
 {
+  tl_assert (mp_handle != 0);
+
   m_http_status = 0;
   m_status = 0;
   m_finished = false;
@@ -764,6 +786,8 @@ void CurlConnection::check () const
 
 void CurlConnection::finished (int status)
 {
+  tl_assert (mp_handle != 0);
+
   if (status != 0) {
     m_status = status;
     m_finished = true;
@@ -1058,10 +1082,12 @@ int CurlNetworkManager::tick ()
 InputHttpStream::InputHttpStream (const std::string &url)
 {
   m_sent = false;
+  m_ready = false;
 
   m_connection.reset (CurlNetworkManager::instance ()->create_connection ());
   m_connection->set_url (url.c_str ());
-  m_connection->finished_event.add (this, &InputHttpStream::finished);
+  m_connection->data_available_event.add (this, &InputHttpStream::on_data_available);
+  m_connection->finished_event.add (this, &InputHttpStream::on_finished);
 }
 
 InputHttpStream::~InputHttpStream ()
@@ -1100,46 +1126,79 @@ InputHttpStream::add_header (const std::string &name, const std::string &value)
 }
 
 void
-InputHttpStream::finished ()
+InputHttpStream::on_finished ()
 {
+  m_progress.reset (0);
   m_ready_event ();
 }
 
+void
+InputHttpStream::on_data_available ()
+{
+  //  send the ready event just once
+  if (! m_ready) {
+    m_data_ready_event ();
+    m_ready = true;
+  }
+}
+
 void
 InputHttpStream::send ()
 {
+  m_ready = false;
+  m_progress.reset (0);
   m_connection->send ();
   m_sent = true;
 }
 
+void
+InputHttpStream::check ()
+{
+  if (m_connection->finished ()) {
+    m_connection->check ();
+  }
+}
+
 size_t
 InputHttpStream::read (char *b, size_t n)
 {
   if (! m_sent) {
-
     send ();
+  }
 
+  //  block until enough data is available
+  {
     //  Prevents deferred methods to be executed during the processEvents below (undesired side effects)
     tl::NoDeferredMethods silent;
 
-    //  TODO: progress, timeout
-    tl::AbsoluteProgress progress (tl::to_string (QObject::tr ("Downloading")) + " " + m_connection->url (), 1);
-    while (! m_connection->finished () && CurlNetworkManager::instance ()->tick ()) {
-      ++progress;
+    if (! m_progress.get ()) {
+      m_progress.reset (new tl::AbsoluteProgress (tl::to_string (QObject::tr ("Downloading")) + " " + m_connection->url (), 1));
     }
 
-    tl_assert (m_connection->finished ());
+    while (n > m_connection->read_available () && ! m_connection->finished () && CurlNetworkManager::instance ()->tick ()) {
+      ++*m_progress;
+    }
+  }
+
+  if (m_connection->finished ()) {
     m_connection->check ();
-
-    if (tl::verbosity() >= 40) {
-      tl::info << "HTTP reponse data read: " << m_connection->read_data_to_string ();
-    }
-
+  } else if (tl::verbosity() >= 40) {
+    tl::info << "HTTP reponse data read: " << m_connection->read_data_to_string ();
   }
 
   return m_connection->fetch_read_data (b, n);
 }
 
+void
+InputHttpStream::close ()
+{
+  m_progress.reset (0);
+  if (m_connection.get ()) {
+    m_connection->close ();
+  }
+  m_sent = m_ready = false;
+}
+
 void
 InputHttpStream::reset ()
 {

src/tl/tl/tlHttpStreamCurl.h

@@ -28,6 +28,7 @@
 #include "tlStream.h"
 #include "tlEvents.h"
 #include "tlObject.h"
+#include "tlProgress.h"
 
 #include <memory>
 
@@ -43,8 +44,7 @@ class HttpCredentialProvider;
  *  Implements the reader from a server using the HTTP protocol
  */
 class TL_PUBLIC InputHttpStream
-  //  NOTE: QObject is required because we use "deleteLater"
-  : public QObject, public tl::Object, public InputStreamBase
+  : public tl::Object, public InputStreamBase
 {
 public:
   /**
@@ -111,19 +111,40 @@ public:
   /**
    *  @brief Gets the "ready" event
    *  Connect to this event for the asynchroneous interface.
+   *  This event is fired when the request has finished.
    */
   tl::Event &ready ()
   {
     return m_ready_event;
   }
 
+  /**
+   *  @brief Checks for errors
+   *  This method can be used after the ready event to check for errors.
+   *  It will throw an exception if errors occured.
+   *  read() will do the same.
+   */
+  void check ();
+
+  /**
+   *  @brief Gets the "data available" event
+   *  Connect to this event for the asynchroneous interface.
+   *  This event is fired when data becomes available for read.
+   *  It is just fired once.
+   */
+  tl::Event &data_ready ()
+  {
+    return m_data_ready_event;
+  }
+
   /**
    *  @brief Gets a value indicating whether data is available
    */
   bool data_available ();
 
+  //  Basic interface
   virtual void reset ();
-
+  virtual void close ();
   virtual std::string source () const;
   virtual std::string absolute_path () const;
   virtual std::string filename () const;
@@ -131,9 +152,13 @@ public:
 private:
   std::auto_ptr<CurlConnection> m_connection;
   tl::Event m_ready_event;
+  tl::Event m_data_ready_event;
   bool m_sent;
+  bool m_ready;
+  std::auto_ptr<tl::AbsoluteProgress> m_progress;
 
-  void finished ();
+  void on_data_available ();
+  void on_finished ();
 };
 
 }

src/tl/tl/tlHttpStreamQt.cc

@@ -115,7 +115,17 @@ InputHttpStream::InputHttpStream (const std::string &url)
 
 InputHttpStream::~InputHttpStream ()
 {
-  // .. nothing yet ..
+  close ();
+}
+
+void
+InputHttpStream::close ()
+{
+  if (mp_active_reply.get ()) {
+    mp_active_reply->abort ();
+    mp_active_reply.release ()->deleteLater ();
+  }
+  mp_reply = 0;
 }
 
 void

src/tl/tl/tlHttpStreamQt.h

@@ -102,6 +102,11 @@ public:
    */
   void send ();
 
+  /**
+   *  @brief Closes the connection
+   */
+  void close ();
+
   /**
    *  @brief Sets the request verb
    *  The default verb is "GET"
@@ -136,6 +141,8 @@ public:
   /**
    *  @brief Gets the "ready" event
    *  Connect to this event for the asynchroneous interface.
+   *  This event is fired when data becomes available or the
+   *  connection has terminated with an error.
    */
   tl::Event &ready ()
   {

src/tl/tl/tlStream.cc

@@ -161,6 +161,8 @@ public:
 
   virtual void reset ();
 
+  virtual void close ();
+
   virtual std::string source () const
   {
     return m_source;
@@ -203,19 +205,12 @@ public:
    */
   virtual ~InputFile ();
 
-  /**
-   *  @brief Read from a file 
-   *
-   *  Implements the basic read method. 
-   *  Will throw a FileReadErrorException if an error occurs.
-   */
   virtual size_t read (char *b, size_t n);
 
-  /**
-   *  @brief Reset to the beginning of the file
-   */
   virtual void reset ();
 
+  virtual void close ();
+
   virtual std::string source () const
   {
     return m_source;
@@ -272,6 +267,11 @@ public:
    */
   virtual void reset ();
 
+  /**
+   *  @brief Closes the pipe
+   */
+  virtual void close ();
+
   /**
    *  @brief Get the source specification (the file name)
    *
@@ -365,15 +365,7 @@ std::string InputStream::absolute_path (const std::string &abstract_path)
 InputStream::~InputStream ()
 {
   if (mp_delegate && m_owns_delegate) {
-    //  NOTE: HTTP stream objects should not be deleted now, since events
-    //  may be pending that deliver the finished signal to the object.
-    tl::InputHttpStream *http = dynamic_cast<tl::InputHttpStream *>(mp_delegate);
-    if (http) {
-      http->ready ().clear ();  //  avoids events from deleted streams
-      http->deleteLater ();
-    } else {
-      delete mp_delegate;
-    }
+    delete mp_delegate;
     mp_delegate = 0;
   }
   if (mp_inflate) {
@@ -502,6 +494,14 @@ InputStream::inflate ()
   mp_inflate = new tl::InflateFilter (*this);
 }
 
+void
+InputStream::close ()
+{
+  if (mp_delegate) {
+    mp_delegate->close ();
+  }
+}
+
 void 
 InputStream::reset ()
 {
@@ -649,12 +649,18 @@ InputFile::InputFile (const std::string &path)
 }
 
 InputFile::~InputFile ()
+{
+  close ();
+}
+
+void
+InputFile::close ()
 {
   if (m_fd >= 0) {
 #if defined(_WIN32)
     _close (m_fd);
 #else
-    close (m_fd);
+    ::close (m_fd);
 #endif
     m_fd = -1;
   }  
@@ -723,6 +729,12 @@ InputZLibFile::InputZLibFile (const std::string &path)
 }
 
 InputZLibFile::~InputZLibFile ()
+{
+  close ();
+}
+
+void
+InputZLibFile::close ()
 {
   if (m_zs != NULL) {
     gzclose (m_zs);
@@ -1171,11 +1183,17 @@ InputPipe::InputPipe (const std::string &path)
 }
 
 InputPipe::~InputPipe ()
+{
+  close ();
+}
+
+void
+InputPipe::close ()
 {
   if (m_file != NULL) {
     fclose (m_file);
     m_file = NULL;
-  }  
+  }
 }
 
 size_t 

src/tl/tl/tlStream.h

@@ -83,6 +83,11 @@ public:
    */
   virtual void reset () = 0;
 
+  /**
+   *  @brief Closes the channel
+   */
+  virtual void close () = 0;
+
   /**
    *  @brief Get the source specification (i.e. the file name)
    */
@@ -136,6 +141,11 @@ public:
     m_pos = 0;
   }
 
+  virtual void close ()
+  {
+    //  .. nothing yet ..
+  }
+
   virtual std::string source () const
   {
     return "data";
@@ -306,6 +316,13 @@ public:
    */
   virtual void reset ();
 
+  /**
+   *  @brief Closes the reader
+   *  This method will finish reading and free resources
+   *  associated with it. HTTP connections will be closed.
+   */
+  void close ();
+
   /**
    *  @brief Gets the absolute path for a given URL
    */

src/tl/tl/tlXMLParser.cc

@@ -106,7 +106,11 @@ public:
         *data++ = *rd;
       }
 
-      return n0 - n;
+      if (n0 == n) {
+        return -1;
+      } else {
+        return n0 - n;
+      }
 
     } catch (tl::Exception &ex) {
       setErrorString (tl::to_qstring (ex.msg ()));