Bug 1170606 part-6: add short read handling to files under mailnews/mime/src draft
authorISHIKAWA, Chiaki <ishikawa@yk.rim.or.jp>
Wed, 01 Jul 2020 13:26:15 +0900
changeset 89801 d9ffe5e92613921c6dc29569ecc38a2b988525bc
parent 89800 76ef617f9a95af7c20ef219079664765e59cbb87
child 89802 ae79e4ab8b2948ee65d92396c4b214e8a36199af
push id11337
push userishikawa@yk.rim.or.jp
push dateWed, 01 Jul 2020 04:26:34 +0000
treeherdertry-comm-central@5b0c8a7b2471 [default view] [failures only]
bugs1170606
Bug 1170606 part-6: add short read handling to files under mailnews/mime/src
mailnews/mime/src/mimedrft.cpp
mailnews/mime/src/mimemrel.cpp
mailnews/mime/src/mimepbuf.cpp
mailnews/mime/src/nsStreamConverter.cpp
--- a/mailnews/mime/src/mimedrft.cpp
+++ b/mailnews/mime/src/mimedrft.cpp
@@ -45,16 +45,20 @@
 #include "nsMsgUtils.h"
 #include "nsCExternalHandlerService.h"
 #include "nsIMIMEService.h"
 #include "nsIMsgAccountManager.h"
 #include "nsMsgBaseCID.h"
 #include "modmimee.h"  // for MimeConverterOutputCallback
 #include "mozilla/mailnews/MimeHeaderParser.h"
 
+/* for logging to Error Console */
+#include "nsIScriptError.h"
+#include "nsIConsoleService.h"
+
 using namespace mozilla::mailnews;
 
 //
 // Header strings...
 //
 #define HEADER_NNTP_POSTING_HOST "NNTP-Posting-Host"
 #define MIME_HEADER_TABLE                        \
   "<TABLE CELLPADDING=0 CELLSPACING=0 BORDER=0 " \
@@ -1383,17 +1387,31 @@ static void mime_parse_stream_complete(n
 
           uint32_t bytesRead;
           nsCOMPtr<nsIInputStream> inputStream;
 
           nsresult rv = NS_NewLocalFileInputStream(getter_AddRefs(inputStream),
                                                    mdd->messageBody->m_tmpFile);
           if (NS_FAILED(rv)) return;
 
-          inputStream->Read(body, bodyLen, &bytesRead);
+          // xxx here we assume that we can read all the file body (fileSize is
+          // set above.) so we should avoid short read and check for errors !
+          rv = FullyReadStream(inputStream, body, bodyLen, &bytesRead);
+          if (NS_FAILED(rv) || bodyLen != bytesRead) {
+#ifdef DEBUG
+            MSG_REPORT_ERROR(rv, "read failed in mime_parse_stream_complete.");
+#endif
+            inputStream->Close();
+            return;  // NS_ERROR_FAILURE;
+                     // XXX FIXME above
+                     // This is a void function and error cannot be
+                     // returned!? Give me a break!  How do we tell the user
+                     // that a serious error occurred internally!
+          }
+          // xxx we ignore error in Close (stream was opened for reading.)
 
           inputStream->Close();
 
           // Convert the body to UTF-8
           char* mimeCharset = nullptr;
           // Get a charset from the header if no override is set.
           if (!charsetOverride)
             mimeCharset = MimeHeaders_get_parameter(
--- a/mailnews/mime/src/mimemrel.cpp
+++ b/mailnews/mime/src/mimemrel.cpp
@@ -1039,16 +1039,19 @@ static int MimeMultipartRelated_parse_eo
     if (NS_FAILED(rv)) {
       PR_Free(buf);
       status = MIME_UNABLE_TO_OPEN_TMP_FILE;
       goto FAIL;
     }
 
     while (1) {
       uint32_t bytesRead = 0;
+      // xxx I think short read is acceptable as long as bytesRead > 0.
+      // But who knows if EINTR processing creeps up at this level.
+      // It is not supposed to happen...
       rv = relobj->input_file_stream->Read(buf, FILE_IO_BUFFER_SIZE - 1,
                                            &bytesRead);
       if (NS_FAILED(rv) || !bytesRead) {
         status = NS_FAILED(rv) ? -1 : 0;
         break;
       } else {
         /* It would be really nice to be able to yield here, and let
            some user events and other input sources get processed.
--- a/mailnews/mime/src/mimepbuf.cpp
+++ b/mailnews/mime/src/mimepbuf.cpp
@@ -227,16 +227,19 @@ int MimePartBufferRead(MimePartBufferDat
     nsresult rv = NS_NewLocalFileInputStream(
         getter_AddRefs(data->input_file_stream), data->file_buffer);
     if (NS_FAILED(rv)) {
       PR_Free(buf);
       return MIME_UNABLE_TO_OPEN_TMP_FILE;
     }
     while (1) {
       uint32_t bytesRead = 0;
+      // xxx I think short read is acceptable as long as bytesRead > 0.
+      // But who knows if EINTR processing creeps up at this level.
+      // It is not supposed to happen...
       rv = data->input_file_stream->Read(buf, buf_size - 1, &bytesRead);
       if (NS_FAILED(rv) || !bytesRead) {
         break;
       } else {
         /* It would be really nice to be able to yield here, and let
         some user events and other input sources get processed.
         Oh well. */
 
--- a/mailnews/mime/src/nsStreamConverter.cpp
+++ b/mailnews/mime/src/nsStreamConverter.cpp
@@ -785,17 +785,37 @@ nsresult nsStreamConverter::OnDataAvaila
   }
 
   nsCOMPtr<nsIInputStream> stream = aIStream;
   NS_ENSURE_TRUE(stream, NS_ERROR_NULL_POINTER);
   char* buf = (char*)PR_Malloc(aLength);
   if (!buf) return NS_ERROR_OUT_OF_MEMORY; /* we couldn't allocate the object */
 
   uint32_t readLen;
-  stream->Read(buf, aLength, &readLen);
+  // xxx This is called on OnDataAvailable.
+  // So we may not want to get blocked, but
+  // still we may want to read as many available octets as possible.
+  // Also perform error check!
+  uint32_t maxcount = aLength;
+  {
+    uint64_t available;
+    nsresult rv = aIStream->Available(&available);
+    if (NS_SUCCEEDED(rv) && available > 0 && maxcount > available)
+      maxcount = available;
+  }
+
+  readLen = maxcount;
+  nsresult rv = FullyReadStream(aIStream, buf, maxcount, &readLen);
+  if (NS_FAILED(rv)) {
+#ifdef DEBUG
+    MSG_REPORT_ERROR(rv, "read failed in nsStreamConverter::OnDataAvailable.");
+#endif
+    PR_FREEIF(buf);
+    return rv;
+  }
 
   // We need to filter out any null characters else we will have a lot of
   // trouble as we use c string everywhere in mime
   char* readPtr;
   char* endPtr = buf + readLen;
 
   // First let see if the stream contains null characters
   for (readPtr = buf; readPtr < endPtr && *readPtr; readPtr++)