Chris Conroy's RTSP followup fixes

3 files changed, 112 insertions, 66 deletions

diff --git a/lib/rtsp.c b/lib/rtsp.c
index d61458c2c..f81f60608 100644
--- a/lib/rtsp.c
+++ b/lib/rtsp.c
@@ -127,6 +127,10 @@ CURLcode Curl_rtsp_done(struct connectdata *conn,
   long CSeq_sent;
   long CSeq_recv;
 
+  /* Bypass HTTP empty-reply checks on receive */
+  if(data->set.rtspreq == RTSPREQ_RECEIVE)
+    premature = TRUE;
+
   httpStatus = Curl_http_done(conn, status, premature);
 
   /* Check the sequence numbers */
@@ -139,7 +143,7 @@ CURLcode Curl_rtsp_done(struct connectdata *conn,
   }
   else if (data->set.rtspreq == RTSPREQ_RECEIVE &&
            (conn->proto.rtspc.rtp_channel == -1)) {
-    infof(data, "Got a non RTP Receive with a CSeq of %ld\n", CSeq_recv);
+    infof(data, "Got an RTP Receive with a CSeq of %ld\n", CSeq_recv);
     /* TODO CPC: Server -> Client logic here */
   }
 
@@ -376,7 +380,7 @@ CURLcode Curl_rtsp(struct connectdata *conn, bool *done)
   result =
     Curl_add_bufferf(req_buffer,
                      "%s %s RTSP/1.0\r\n" /* Request Stream-URI RTSP/1.0 */
-                     "CSeq: %d \r\n", /* CSeq */
+                     "CSeq: %d\r\n", /* CSeq */
                      (p_request ? p_request : ""), p_stream_uri,
                      rtsp->CSeq_sent);
   if(result)
@@ -387,7 +391,7 @@ CURLcode Curl_rtsp(struct connectdata *conn, bool *done)
    * to make comparison easier
    */
   if(p_session_id) {
-    result = Curl_add_bufferf(req_buffer, "Session: %s \r\n", p_session_id);
+    result = Curl_add_bufferf(req_buffer, "Session: %s\r\n", p_session_id);
     if(result)
       return result;
   }
@@ -512,8 +516,7 @@ CURLcode Curl_rtsp(struct connectdata *conn, bool *done)
 CURLcode Curl_rtsp_rtp_readwrite(struct SessionHandle *data,
                                  struct connectdata *conn,
                                  ssize_t *nread,
-                                 bool *readmore,
-                                 bool *done) {
+                                 bool *readmore) {
   struct SingleRequest *k = &data->req;
   struct rtsp_conn *rtspc = &(conn->proto.rtspc);
 
@@ -538,10 +541,6 @@ CURLcode Curl_rtsp_rtp_readwrite(struct SessionHandle *data,
     rtp_dataleft = *nread;
   }
 
-  if(rtp_dataleft == 0 || rtp[0] != '$') {
-    return  CURLE_OK;
-  }
-
   while((rtp_dataleft > 0) &&
         (rtp[0] == '$')) {
     if(rtp_dataleft > 4) {
@@ -564,22 +563,18 @@ CURLcode Curl_rtsp_rtp_readwrite(struct SessionHandle *data,
       else {
         /* We have the full RTP interleaved packet
          * Write out the header but strip the leading '$' */
-        infof(data, "CPCDEBUG: RTP write channel %d rtp_length %d\n",
-              rtspc->rtp_channel, rtp_length);
+        DEBUGF(infof(data, "RTP write channel %d rtp_length %d\n",
+              rtspc->rtp_channel, rtp_length));
         result = rtp_client_write(conn, &rtp[1], rtp_length + 3);
         if(result) {
-            failf(data, "Got an error writing an RTP packet");
-            *done = TRUE;
-            *readmore = FALSE;
-            return result;
+          failf(data, "Got an error writing an RTP packet");
+          *readmore = FALSE;
+          Curl_safefree(rtspc->rtp_buf);
+          rtspc->rtp_buf = NULL;
+          rtspc->rtp_bufsize = 0;
+          return result;
         }
 
-        /* Update progress */
-        k->bytecount += rtp_length + 4;
-        Curl_pgrsSetDownloadCounter(data, k->bytecount);
-        if(k->bytecountp)
-          *k->bytecountp = k->bytecount;
-
         /* Move forward in the buffer */
         rtp_dataleft -= rtp_length + 4;
         rtp += rtp_length + 4;
@@ -587,11 +582,8 @@ CURLcode Curl_rtsp_rtp_readwrite(struct SessionHandle *data,
         if(data->set.rtspreq == RTSPREQ_RECEIVE) {
           /* If we are in a passive receive, give control back
            * to the app as often as we can.
-           *
-           * Otherwise, keep chugging along until we get RTSP data
            */
           k->keepon &= ~KEEP_RECV;
-          *done = TRUE;
         }
       }
     }
@@ -602,41 +594,36 @@ CURLcode Curl_rtsp_rtp_readwrite(struct SessionHandle *data,
     }
   }
 
-  if(*done || *readmore) {
-    if(rtp_dataleft != 0 && rtp[0] == '$') {
-      infof(data, "RTP Rewinding %zu %s %s\n", rtp_dataleft,
-            *done ? "DONE " : "",
-            *readmore ? "READMORE" : "");
+  if(rtp_dataleft != 0 && rtp[0] == '$') {
+    DEBUGF(infof(data, "RTP Rewinding %zu %s\n", rtp_dataleft,
+          *readmore ? "(READMORE)" : ""));
 
-      /* Store the incomplete RTP packet for a "rewind" */
-      scratch = malloc(rtp_dataleft);
-      if(!scratch)
-        return CURLE_OUT_OF_MEMORY;
-      memcpy(scratch, rtp, rtp_dataleft);
-      Curl_safefree(rtspc->rtp_buf);
-      rtspc->rtp_buf = scratch;
-      rtspc->rtp_bufsize = rtp_dataleft;
-      return CURLE_OK;
-    }
+    /* Store the incomplete RTP packet for a "rewind" */
+    scratch = malloc(rtp_dataleft);
+    if(!scratch)
+      return CURLE_OUT_OF_MEMORY;
+    memcpy(scratch, rtp, rtp_dataleft);
+    Curl_safefree(rtspc->rtp_buf);
+    rtspc->rtp_buf = scratch;
+    rtspc->rtp_bufsize = rtp_dataleft;
+
+    /* As far as the transfer is concerned, this data is consumed */
+    *nread = 0;
+    return CURLE_OK;
   }
   else {
-    /* RTP followed by RTSP */
-    if(rtp_dataleft == 0) {
-      /* Need more */
-      *readmore = TRUE;
-    }
-    else {
-      /* Fix up k->str to point just after the last RTP packet */
-      k->str += *nread - rtp_dataleft;
+    /* Fix up k->str to point just after the last RTP packet */
+    k->str += *nread - rtp_dataleft;
 
-      /* rtp may point into the leftover buffer, but at this point
-       * it is somewhere in the merged data from k->str. */
+    /* either all of the data has been read or...
+     * rtp now points at the next byte to parse
+     */
+    if(rtp_dataleft > 0)
       DEBUGASSERT(k->str[0] == rtp[0]);
 
-      DEBUGASSERT(rtp_dataleft < *nread); /* sanity check */
+    DEBUGASSERT(rtp_dataleft <= *nread); /* sanity check */
 
-      *nread = rtp_dataleft;
-    }
+    *nread = rtp_dataleft;
   }
 
   /* If we get here, we have finished with the leftover/merge buffer */
@@ -644,9 +631,6 @@ CURLcode Curl_rtsp_rtp_readwrite(struct SessionHandle *data,
   rtspc->rtp_buf = NULL;
   rtspc->rtp_bufsize = 0;
 
-  /* TODO CPC: Could implement parsing logic for Server->Client requests
-     here */
-
   return CURLE_OK;
 }
 
@@ -701,7 +685,7 @@ CURLcode Curl_rtsp_parseheader(struct connectdata *conn,
       failf(data, "Unable to read the CSeq header: [%s]", header);
       return CURLE_RTSP_CSEQ_ERROR;
     }
-    }
+  }
   else if(checkprefix("Session:", header)) {
     char *start;
 
diff --git a/lib/rtsp.h b/lib/rtsp.h
index dcb50d3b0..52ad7dbd6 100644
--- a/lib/rtsp.h
+++ b/lib/rtsp.h
@@ -27,11 +27,17 @@
 
 extern const struct Curl_handler Curl_handler_rtsp;
 
+/*
+ * Parse and write out any available RTP data.
+ *
+ * nread: amount of data left after k->str. will be modified if RTP
+ *        data is parsed and k->str is moved up
+ * readmore: whether or not the RTP parser needs more data right away
+ */
 CURLcode Curl_rtsp_rtp_readwrite(struct SessionHandle *data,
                                  struct connectdata *conn,
                                  ssize_t *nread,
-                                 bool *readmore,
-                                 bool *done);
+                                 bool *readmore);
 
 
 /* protocol-specific functions set up to be called by the main engine */
diff --git a/lib/transfer.c b/lib/transfer.c
index a59588853..915d1de97 100644
--- a/lib/transfer.c
+++ b/lib/transfer.c
@@ -368,7 +368,11 @@ static CURLcode readwrite_data(struct SessionHandle *data,
 {
   CURLcode result = CURLE_OK;
   ssize_t nread; /* number of bytes read */
+  size_t excess = 0; /* excess bytes read */
   bool is_empty_data = FALSE;
+#ifndef CURL_DISABLE_RTSP
+  bool readmore = FALSE; /* used by RTP to signal for more data */
+#endif
 
   *done = FALSE;
 
@@ -437,15 +441,13 @@ static CURLcode readwrite_data(struct SessionHandle *data,
     k->str = k->buf;
 
 #ifndef CURL_DISABLE_RTSP
+    /* Check for RTP at the beginning of the data */
     if(conn->protocol & PROT_RTSP) {
-      bool readmore = FALSE;
-      result = Curl_rtsp_rtp_readwrite(data, conn, &nread, &readmore, done);
+      result = Curl_rtsp_rtp_readwrite(data, conn, &nread, &readmore);
       if(result)
         return result;
       if(readmore)
         break;
-      if(*done)
-        return CURLE_OK;
     }
 #endif
 
@@ -458,6 +460,18 @@ static CURLcode readwrite_data(struct SessionHandle *data,
       result = Curl_http_readwrite_headers(data, conn, &nread, &stop_reading);
       if(result)
         return result;
+
+#ifndef CURL_DISABLE_RTSP
+      /* Check for RTP after the headers if there is no Content */
+      if(k->maxdownload <= 0 && nread > 0 && (conn->protocol & PROT_RTSP)) {
+        result = Curl_rtsp_rtp_readwrite(data, conn, &nread, &readmore);
+        if(result)
+          return result;
+        if(readmore)
+          break;
+      }
+#endif
+
       if(stop_reading)
         /* We've stopped dealing with input, get out of the do-while loop */
         break;
@@ -594,13 +608,19 @@ static CURLcode readwrite_data(struct SessionHandle *data,
       }
 #endif   /* CURL_DISABLE_HTTP */
 
+      /* Account for body content stored in the header buffer */
+      if(k->badheader && !k->ignorebody) {
+        DEBUGF(infof(data, "Increasing bytecount by %" FORMAT_OFF_T" from hbuflen\n", k->hbuflen));
+        k->bytecount += k->hbuflen;
+      }
+
       if((-1 != k->maxdownload) &&
          (k->bytecount + nread >= k->maxdownload)) {
 
+        excess = (size_t)(k->bytecount + nread - k->maxdownload);
         if(conn->data->multi && Curl_multi_canPipeline(conn->data->multi)) {
           /* The 'excess' amount below can't be more than BUFSIZE which
              always will fit in a size_t */
-          size_t excess = (size_t)(k->bytecount + nread - k->maxdownload);
           if(excess > 0 && !k->ignorebody) {
             infof(data,
                   "Rewinding stream by : %d"
@@ -612,6 +632,15 @@ static CURLcode readwrite_data(struct SessionHandle *data,
             read_rewind(conn, excess);
           }
         }
+        else {
+          infof(data,
+                "Excess found in a non pipelined read:"
+                " excess=%zu"
+                ", size=%" FORMAT_OFF_T
+                ", maxdownload=%" FORMAT_OFF_T
+                ", bytecount=%" FORMAT_OFF_T "\n",
+                excess, k->size, k->maxdownload, k->bytecount);
+        }
 
         nread = (ssize_t) (k->maxdownload - k->bytecount);
         if(nread < 0 ) /* this should be unusual */
@@ -630,9 +659,17 @@ static CURLcode readwrite_data(struct SessionHandle *data,
         if(k->badheader && !k->ignorebody) {
           /* we parsed a piece of data wrongly assuming it was a header
              and now we output it as body instead */
-          result = Curl_client_write(conn, CLIENTWRITE_BODY,
-                                     data->state.headerbuff,
-                                     k->hbuflen);
+
+          /* Don't let excess data pollute body writes */
+          if(k->maxdownload == -1 || (curl_off_t)k->hbuflen <= k->maxdownload)
+            result = Curl_client_write(conn, CLIENTWRITE_BODY,
+                data->state.headerbuff,
+                k->hbuflen);
+          else
+            result = Curl_client_write(conn, CLIENTWRITE_BODY,
+                data->state.headerbuff,
+                k->maxdownload);
+
           if(result)
             return result;
         }
@@ -694,6 +731,25 @@ static CURLcode readwrite_data(struct SessionHandle *data,
 
     } /* if(! header and data to read ) */
 
+#ifndef CURL_DISABLE_RTSP
+    if(excess > 0 && !conn->bits.stream_was_rewound &&
+        (conn->protocol & PROT_RTSP)) {
+      /* Check for RTP after the content if there is unrewound excess */
+
+      /* Parse the excess data */
+      k->str += nread;
+      nread = excess;
+
+      result = Curl_rtsp_rtp_readwrite(data, conn, &nread, &readmore);
+      if(result)
+        return result;
+
+      if(readmore)
+        k->keepon |= KEEP_RECV; /* we're not done reading */
+        break;
+    }
+#endif
+
     if(is_empty_data) {
       /* if we received nothing, the server closed the connection and we
          are done */