[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-core-devel
Subject:    PATCH: 1st part of the patch to solve the "large file upload
From:       "Dawit A." <adawit () kde ! org>
Date:       2003-06-06 4:07:00
[Download RAW message or body]

Hi,

The following patches are intended to be the beginning step to solving an old 
and  well known issue of uploading large files through KIO::post(...). It 
mainly affects webdav and kio_http. See  bug:34578.

For the benefit of those who have not been following this issue. See 
http://lists.kde.org/?l=kfm-devel&m=103851085211923&w=2 for details.

Anyways, what is left to completely fix this problem once this stuff is 
committed ?

- Changes in khtml to make it store the form to be posted in a temporary file 
and pass around this name through the new URLArgs::setPostDataFilename. This 
is easier said than done.  Perhaps one of the khtml developers can help me 
out here ?

- Minor changes in konqueror (konq_view.*) to store the filename instead of a 
shallow copy of the buffer.

- Changes to kio_http, which I have already done and will provide a patch for 
shortly.  It was not included here because I need to break out the rather 
large changes I have for kio_http into smaller chunks...

Comments ? Feedback ?

Regards,
Dawit A.

["browserextension-20030606-1.diff" (text/x-diff)]

Index: browserextension.cpp
===================================================================
RCS file: /home/kde/kdelibs/kparts/browserextension.cpp,v
retrieving revision 1.52
diff -u -p -b -B -w -r1.52 browserextension.cpp
--- browserextension.cpp	26 May 2003 22:58:36 -0000	1.52
+++ browserextension.cpp	6 Jun 2003 04:05:04 -0000
@@ -70,8 +70,11 @@ struct URLArgsPrivate
       lockHistory = false;
       newTab = false;
     }
+
     QString contentType; // for POST
+    QString dataFilename;
     QMap<QString, QString> metaData;
+
     bool doPost;
     bool redirectedRequest;
     bool lockHistory;
@@ -146,6 +149,13 @@ void URLArgs::setRedirectedRequest( bool
   d->redirectedRequest = redirected;
 }
 
+void URLArgs::setPostDataFilename ( const QString& name )
+{
+  if (!d)
+    d = new URLArgsPrivate;
+  d->dataFilename = name;
+}
+
 bool URLArgs::redirectedRequest () const
 {
   return d ? d->redirectedRequest : false;
@@ -156,6 +166,11 @@ QString URLArgs::contentType() const
   return d ? d->contentType : QString::null;
 }
 
+QString URLArgs::postDataFilename ()
+{
+   return d ? d->dataFilename : QString::null;
+}
+
 QMap<QString, QString> &URLArgs::metaData()
 {
   if (!d)
Index: browserextension.h
===================================================================
RCS file: /home/kde/kdelibs/kparts/browserextension.h,v
retrieving revision 1.102
diff -u -p -b -B -w -r1.102 browserextension.h
--- browserextension.h	14 May 2003 21:50:46 -0000	1.102
+++ browserextension.h	6 Jun 2003 04:05:04 -0000
@@ -90,6 +90,8 @@ struct URLArgs
 
   /**
    * KHTML-specific field, contents of the HTTP POST data.
+   *
+   * @deprecated. Use @ref setPostDataFile instead.
    */
   QByteArray postData;
   
@@ -114,6 +118,20 @@ struct URLArgs
   bool doPost() const;
 
   /**
+   * KHTML specific data. Sets the name of the file that contains
+   * HTTP POST data.
+   *
+   * @param name filename that contains the data to be transmitted.
+   */
+  void setPostDataFilename ( const QString& name );
+
+  /**
+   * @return the filename of the data to be transmitted to a remote
+   * or local server.
+   */
+  QString postDataFilename ();
+
+  /**
    * Whether to lock the history when opening the next URL.
    * This is used during e.g. a redirection, to avoid a new entry
    * in the history.
@@ -146,14 +164,13 @@ struct URLArgs
   bool trustedSource;
 
   /**
-   * @return true if the request was a result of a META refresh/redirect request or
-   * HTTP redirect.
+   * @return true if the request was a result of a META tag or HTTP redirect.
    */
   bool redirectedRequest () const;
 
   /**
-   * Set the redirect flag to indicate URL is a result of either a META redirect
-   * or HTTP redirect.
+   * Set the redirect flag to indicate URL is a result of either an HTML
+   * META tag or HTTP redirect.
    *
    * @param redirected
    */

["job-20030606-1.diff" (text/x-diff)]

Index: jobclasses.h
===================================================================
RCS file: /home/kde/kdelibs/kio/kio/jobclasses.h,v
retrieving revision 1.118
diff -u -p -b -B -w -r1.118 jobclasses.h
--- jobclasses.h	25 May 2003 16:29:05 -0000	1.118
+++ jobclasses.h	6 Jun 2003 04:04:12 -0000
@@ -811,7 +812,7 @@ namespace KIO {
          *
 	 * @param job the job that emitted this signal
          * @param data buffer to fill with data to send to the
-         * slave. An empty buffer indicates end of data. (EOD)
+         * slave. An empty buffer indicates end of data (EOD).
          */
         void dataReq( KIO::Job *job, QByteArray &data);
 
@@ -1541,6 +1550,105 @@ namespace KIO {
 	class DeleteJobPrivate* d;
     };
 
-}
+    /**
+     * Given a data source, stream its content to the remote machine.
+     *
+     * This job, Unlike its parent, streams the source data at 256K to
+     * io-slaves no matter how the data is supplied to it. It accepts
+     * three types of data sources: a file, a buffer and a pointer to
+     * a QIODevice.
+     *
+     * The buffer constructor is provided for backwards compatability
+     * with the parent's API. It is only appropriate to use it if you
+     * have small data (< 256 KB) to stream to the remote machine. For
+     * streaming large data use either one of the other two constructors.
+     *
+     * @since 3.2
+     * @author Dawit Alemayehu <adawit@kde.org>
+     */
+    class PostJob : public TransferJob {
+    Q_OBJECT
+
+    public:
+        /**
+         * Constructs a job that will stream the contents pointed to by the
+         * QIODevice.
+         */
+        PostJob (const KURL& url, int command, const QByteArray& packedArgs,
+                 QIODevice * device, bool showProgressInfo);
+        /**
+         * Constructs a job that will stream the contents of the given filename
+         * to remote machine.
+         */
+        PostJob (const KURL& url, int command, const QByteArray& packedArgs,
+                 const QString& filename, bool showProgressInfo);
+
+
+        /**
+         * @deprecated. This ctor is only provided for compatability sake
+         * until all protocols that use KIO::http_post are ported to use
+         * the newer API above. You should use either one of the other two
+         * ctors instead.
+         */
+        PostJob (const KURL& url, int command, const QByteArray& packedArgs,
+                 const QByteArray& data, bool showProgressInfo);
+        /**
+         * Destructor.
+         */
+        ~PostJob ();
+
+        /**
+         * Re-implemented for internal reasons.  API is unaffected.
+         *
+         * Note that the job will fail and return error if the POST action
+         * is being done to a non-safe port and port checking has been enabled.
+         * See @ref setEnablePortCheck.
+         */
+        virtual void start (Slave* slave);
+
+        /**
+         * Enable port number validation before streaming the data to the remote
+         * machine.  This sanity check is mainly provided to protect against
+         * malicious redirection of HTTP POST requests to unsafe (undesirable)
+         * ports on a remote machine.
+         *
+         * @param enable if true validate the port before starting a job. Do
+         *               not perform any checks otherwise.
+         */
+        void setEnablePortCheck (bool enable);
+
+        /**
+         * @return true if non-safe port checking has been enabled, false otherwise.
+         */
+        bool isPortCheckEnabled () const;
+
+    protected:
+        /**
+         * Verifies whether or not the requested action (POST) is being done
+         * to a "safe" (non-malicious) port number.
+         *
+         * @return true if port number is safe, false otherwise.
+         */
+        virtual bool isPortSafe ();
+
+
+    protected slots:
+        /**
+         * Re-implemented for internal reasons. API remains unaffected.
+         */
+        virtual void slotDataReq ();
+
+    private:
+        KIO::filesize_t _bytesSent;
+        KIO::filesize_t _blockSize;
+
+        bool _validatePort;
+        bool _autoDeleteSource;
+
+        QString _tempfile;
+        QIODevice* _dataSource;
+        class PostJobPrivate* d;
+    };
+};
 
 #endif
Index: job.cpp
===================================================================
RCS file: /home/kde/kdelibs/kio/kio/job.cpp,v
retrieving revision 1.347
diff -u -p -b -B -w -r1.347 job.cpp
--- job.cpp	25 May 2003 16:29:05 -0000	1.347
+++ job.cpp	6 Jun 2003 04:04:14 -0000
@@ -38,6 +38,8 @@ extern "C" {
 }
 #include <qtimer.h>
 #include <qfile.h>
+#include <qbuffer.h>
+#include <qiodevice.h>
 
 #include <kapplication.h>
 #include <kglobal.h>
@@ -71,6 +73,9 @@ template class QPtrList<KIO::Job>;
 //this will update the report dialog with 5 Hz, I think this is fast enough, aleXXX
 #define REPORT_TIMEOUT 200
 
+// Buffer size for streaming (UPLOADING) data to remote machine.
+#define UPLOAD_BUF_SIZE       1024*256      // 256 KB
+
 #define KIO_ARGS QByteArray packedArgs; QDataStream stream( packedArgs, IO_WriteOnly \
); stream  
 class Job::JobPrivate
@@ -205,18 +210,27 @@ void Job::emitResult()
 
 void Job::kill( bool quietly )
 {
-  kdDebug(7007) << "Job::kill this=" << this << " m_progressId=" << m_progressId << \
" quietly=" << quietly << endl; +  kdDebug(7007) << "Job::kill this=" << this << " \
m_progressId=" << m_progressId +                << " quietly=" << quietly << endl;
+
   // kill all subjobs, without triggering their result slot
   QPtrListIterator<Job> it( subjobs );
+
   for ( ; it.current() ; ++it )
      (*it)->kill( true );
+
   subjobs.clear();
 
-  if ( ! quietly ) {
+  if ( ! quietly )
+  {
+    // Only set the error condition, if one has not already been provided.
+    if (!m_error)
     m_error = ERR_USER_CANCELED;
+
     emit canceled( this ); // Not very useful (deprecated)
     emitResult();
-  } else
+  }
+  else
   {
     if ( m_progressId ) // in both cases we want to hide the progress window
       Observer::self()->jobFinished( m_progressId );
@@ -1069,127 +1084,30 @@ TransferJob *KIO::get( const KURL& url, 
     return job;
 }
 
-class PostErrorJob : public TransferJob
-{
-public:
-
-  // KDE 4: Make it const QString & 
-  PostErrorJob(QString url, const QByteArray &packedArgs, const QByteArray \
&postData, bool showProgressInfo) : TransferJob("", CMD_SPECIAL, packedArgs, \
                postData, showProgressInfo)
-  {
-    m_error = KIO::ERR_POST_DENIED;
-    m_errorText = url;
-  }
-
-};
-
 TransferJob *KIO::http_post( const KURL& url, const QByteArray &postData, bool \
showProgressInfo )  {
-    bool valid = true;
-
-    // filter out non https? protocols
-    if ((url.protocol() != "http") && (url.protocol() != "https" ))
-        valid = false;
-
-    // filter out some malicious ports
-    static const int bad_ports[] = {
-        1,   // tcpmux
-        7,   // echo
-        9,   // discard
-        11,   // systat
-        13,   // daytime
-        15,   // netstat
-        17,   // qotd
-        19,   // chargen
-        20,   // ftp-data
-        21,   // ftp-cntl
-        22,   // ssh
-        23,   // telnet
-        25,   // smtp
-        37,   // time
-        42,   // name
-        43,   // nicname
-        53,   // domain
-        77,   // priv-rjs
-        79,   // finger
-        87,   // ttylink
-        95,   // supdup
-        101,  // hostriame
-        102,  // iso-tsap
-        103,  // gppitnp
-        104,  // acr-nema
-        109,  // pop2
-        110,  // pop3
-        111,  // sunrpc
-        113,  // auth
-        115,  // sftp
-        117,  // uucp-path
-        119,  // nntp
-        123,  // NTP
-        135,  // loc-srv / epmap
-        139,  // netbios
-        143,  // imap2
-        179,  // BGP
-        389,  // ldap
-        512,  // print / exec
-        513,  // login
-        514,  // shell
-        515,  // printer
-        526,  // tempo
-        530,  // courier
-        531,  // Chat
-        532,  // netnews
-        540,  // uucp
-        556,  // remotefs
-        587,  // sendmail
-        601,  //
-        989,  // ftps data
-        990,  // ftps
-        992,  // telnets
-        993,  // imap/SSL
-        995,  // pop3/SSL
-        1080, // SOCKS
-        2049, // nfs
-        4045, // lockd
-        6000, // x11
-        6667, // irc
-        0};
-    for (int cnt=0; bad_ports[cnt]; ++cnt)
-        if (url.port() == bad_ports[cnt])
-        {
-            valid = false;
-            break;
-        }
-
-    if( !valid )
-    {
-	static bool override_loaded = false;
-	static QValueList< int >* overriden_ports = NULL;
-	if( !override_loaded )
-	{
-	    KConfig cfg( "kio_httprc", true );
-	    overriden_ports = new QValueList< int >;
-	    *overriden_ports = cfg.readIntListEntry( "OverriddenPorts" );
-	    override_loaded = true;
-	}
-	for( QValueList< int >::ConstIterator it = overriden_ports->begin();
-	     it != overriden_ports->end();
-	     ++it )
-	if( overriden_ports->contains( url.port()))
-	    valid = true;
+    // Send http post command (1), decoded path and encoded query
+    KIO_ARGS << static_cast<int>(1) << url;
+    PostJob * job = new PostJob( url, CMD_SPECIAL, packedArgs, postData,
+                                 showProgressInfo );
+    return job;
     }
 
-    // if request is not valid, return an invalid transfer job
-    if (!valid)
+TransferJob *KIO::http_post( const KURL& url, QIODevice* dataSource, bool \
showProgressInfo )  {
-        KIO_ARGS << (int)1 << url;
-        TransferJob * job = new PostErrorJob(url.url(), packedArgs, postData, \
showProgressInfo); +    // Send http post command (1), decoded path and encoded query
+    KIO_ARGS << static_cast<int>(1) << url;
+    PostJob * job = new PostJob( url, CMD_SPECIAL, packedArgs, dataSource,
+                                 showProgressInfo );
         return job;
     }
 
+TransferJob *KIO::http_post( const KURL& url, const QString& filename, bool \
showProgressInfo ) +{
     // Send http post command (1), decoded path and encoded query
-    KIO_ARGS << (int)1 << url;
-    TransferJob * job = new TransferJob( url, CMD_SPECIAL,
-                                         packedArgs, postData, showProgressInfo );
+    KIO_ARGS << static_cast<int>(1) << url;
+    PostJob * job = new PostJob( url, CMD_SPECIAL, packedArgs, filename,
+                                 showProgressInfo );
     return job;
 }
 
@@ -3703,6 +3619,281 @@ MultiGetJob *KIO::multi_get(long id, con
     MultiGetJob * job = new MultiGetJob( url, false );
     job->get(id, url, metaData);
     return job;
+}
+
+
+
+PostJob::PostJob (const KURL& url, int command, const QByteArray& packedArgs,
+                      QIODevice * source, bool showProgressInfo)
+          :TransferJob (url, command, packedArgs, QByteArray(), showProgressInfo),
+           _bytesSent(0), _blockSize (UPLOAD_BUF_SIZE), _validatePort (false),
+           _autoDeleteSource (false), _dataSource (source)
+{
+}
+
+PostJob::PostJob (const KURL& url, int command, const QByteArray& packedArgs,
+                      const QString& filename, bool showProgressInfo)
+          :TransferJob (url, command, packedArgs, QByteArray(), showProgressInfo),
+           _bytesSent(0), _blockSize (UPLOAD_BUF_SIZE), _validatePort (false),
+           _autoDeleteSource (true)
+
+{
+  _dataSource = new QFile (filename);
+}
+
+PostJob::PostJob (const KURL& url, int command, const QByteArray& packedArgs,
+                      const QByteArray& data, bool showProgressInfo)
+          :TransferJob (url, command, packedArgs, QByteArray(), showProgressInfo),
+           _bytesSent(0), _blockSize (UPLOAD_BUF_SIZE), _validatePort (false),
+           _autoDeleteSource (true)
+
+{
+  if (data.size() < UPLOAD_BUF_SIZE)
+  {
+    _dataSource = new QBuffer (data);
+  }
+  else
+  {
+    KTempFile temp (QString::null, QString::null, 0600);
+    temp.close ();
+    _tempfile = temp.name ();
+    _dataSource = new QFile (_tempfile);
+
+    if (data.size() && _dataSource->open (IO_WriteOnly))
+    {
+      KIO::filesize_t offset = 0;
+      KIO::filesize_t segment;
+
+      while (offset < data.size())
+      {
+        segment = (data.size () < _blockSize) ? data.size ():_blockSize;
+        _dataSource->writeBlock (data.data() + offset, segment);
+        offset += segment;
+      }
+
+      // Close it to force a flush. It will be re-opened when
+      // start is invoked...
+      _dataSource->close();
+    }
+  }
+}
+
+PostJob::~PostJob ()
+{
+  if (_autoDeleteSource && _dataSource)
+  {
+    kdDebug(7007) << "PostJob dtor: Deleting pointer to data source" << endl;
+    _dataSource->close ();
+    delete _dataSource;
+    _dataSource = 0;
+  }
+
+  // Delete the temporary file if we had created one before...
+  if (!_tempfile.isEmpty())
+    QFile::remove (_tempfile);
+}
+
+bool PostJob::isPortSafe()
+{
+  bool valid = true;
+
+  // filter out non https? protocols
+  if ((m_url.protocol() != "http") && (m_url.protocol() != "https" ))
+    valid = false;
+
+  // filter out some malicious ports
+  static const int bad_ports[] =
+  {
+    1,   // tcpmux
+    7,   // echo
+    9,   // discard
+    11,   // systat
+    13,   // daytime
+    15,   // netstat
+    17,   // qotd
+    19,   // chargen
+    20,   // ftp-data
+    21,   // ftp-cntl
+    22,   // ssh
+    23,   // telnet
+    25,   // smtp
+    37,   // time
+    42,   // name
+    43,   // nicname
+    53,   // domain
+    77,   // priv-rjs
+    79,   // finger
+    87,   // ttylink
+    95,   // supdup
+    101,  // hostriame
+    102,  // iso-tsap
+    103,  // gppitnp
+    104,  // acr-nema
+    109,  // pop2
+    110,  // pop3
+    111,  // sunrpc
+    113,  // auth
+    115,  // sftp
+    117,  // uucp-path
+    119,  // nntp
+    123,  // NTP
+    135,  // loc-srv / epmap
+    139,  // netbios
+    143,  // imap2
+    179,  // BGP
+    389,  // ldap
+    512,  // print / exec
+    513,  // login
+    514,  // shell
+    515,  // printer
+    526,  // tempo
+    530,  // courier
+    531,  // Chat
+    532,  // netnews
+    540,  // uucp
+    556,  // remotefs
+    587,  // sendmail
+    601,  //
+    989,  // ftps data
+    990,  // ftps
+    992,  // telnets
+    993,  // imap/SSL
+    995,  // pop3/SSL
+    1080, // SOCKS
+    2049, // nfs
+    4045, // lockd
+    6000, // x11
+    6667, // irc
+    0
+  };
+
+  for (int cnt=0; bad_ports[cnt]; ++cnt)
+  {
+    if (m_url.port() == bad_ports[cnt])
+    {
+      valid = false;
+      break;
+    }
+  }
+
+  if( !valid )
+  {
+    static bool override_loaded = false;
+    static QValueList< int >* overriden_ports = NULL;
+
+    if( !override_loaded )
+    {
+      KConfig cfg( "kio_httprc", true );
+      overriden_ports = new QValueList< int >;
+      *overriden_ports = cfg.readIntListEntry( "OverriddenPorts" );
+      override_loaded = true;
+    }
+
+    for (QValueList< int >::ConstIterator it = overriden_ports->begin();
+        it != overriden_ports->end(); ++it)
+    {
+      if( overriden_ports->contains(m_url.port()))
+        valid = true;
+    }
+  }
+
+  // if request is not valid, return an invalid transfer job
+  if (!valid)
+  {
+    m_error = KIO::ERR_POST_DENIED;
+    m_errorText = m_url.url();
+  }
+
+  return valid;
+}
+
+void PostJob::start (Slave* slave)
+{
+  assert (_dataSource);
+
+  if (_validatePort && !isPortSafe())
+  {
+    slotFinished ();
+    return;
+  }
+
+  if (!_dataSource->isOpen() && !_dataSource->open (IO_ReadOnly))
+  {
+    kdDebug (7007) << "PostJob::start: Unable to open data source!!" << endl;
+
+    m_error = KIO::ERR_INTERNAL;
+    m_errorText = m_url.url();
+
+    slotFinished();
+    return;
+  }
+
+  // Set the data size...
+  m_totalSize = _dataSource->size();
+  addMetaData ("content-length", KIO::number (m_totalSize));
+
+  TransferJob::start (slave);
+}
+
+void PostJob::slotDataReq()
+{
+  Q_ASSERT(_dataSource);
+
+  // If we reached the end of the device, indicate the condition
+  // to the io-slave and reset the device position to 0 so that
+  // the data can be re-transmitted if the io-slave requests it
+  // again.
+  if (_dataSource->atEnd())
+  {
+    kdDebug (7007) << "PostJob::slotDataReq: DONE transferring data..." << endl;
+    _dataSource->reset();
+    staticData = QByteArray();
+    _bytesSent = 0;
+  }
+  else
+  {
+      KIO::filesize_t dataSize = _blockSize;
+
+      if ((m_totalSize - _bytesSent) < dataSize)
+        dataSize = m_totalSize - _bytesSent;
+
+      staticData.fill (0, dataSize);
+
+      if( _dataSource->readBlock (staticData.data(), dataSize) < 0 )
+      {
+        m_error = ERR_INTERNAL;
+        m_errorText = i18n("PostJob::slotDataReq: Could not read from data \
source."); +        kill ();
+
+        kdDebug(7007) << "PostJob::slotDataReq: Could not read from data source." << \
endl; +        return;
+      }
+
+      _bytesSent += dataSize;
+      kdDebug (7007) << "PostJob::slotDataReq: Uploaded " << KIO::number(_bytesSent)
+                     << " of " << KIO::number(m_totalSize) << " bytes..." << endl;
+  }
+
+  m_slave->send( MSG_DATA, staticData );
+
+  // Copied from TransferJob::slotDataReq. We do not call the parent class
+  // directly to avoid checks that are no longer necessary here.
+  if( m_subJob )
+  {
+    // Bitburger protocol in action
+    suspend(); // Wait for more data from subJob.
+    m_subJob->resume(); // Ask for more!
+  }
+}
+
+void PostJob::setEnablePortCheck (bool enable)
+{
+  _validatePort = enable;
+}
+
+bool PostJob::isPortCheckEnabled () const
+{
+  return _validatePort;
 }
 
 



[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic