ref: e2d7b266a0cbb09203d1d2fc547ebbd73e293fe6
parent: 800be8c0a071327ff937850cf66d97cd264d72a0
author: Timothy B. Terriberry <[email protected]>
date: Sat Oct 20 10:12:46 EDT 2012
Re-do abstract stream reader API.
This changes op_read_func to
a) Take a single byte count to read instead of an "item" count
(which the http backend couldn't properly support anyway).
b) Use integers for buffer sizes to avoid having to worry about
sign differences and whether size_t is larger or smaller than
opus_int64, etc.
c) Return an explicit error code (instead of using errno like
fread).
We had already eliminated the use of errno, but we did it by
treating read errors and EOF identically in all cases.
This was preventing us from reporting SSL truncation attacks
from the https backend.
The https backend now properly reports such errors.
This commit also fixes a bug introduced in 9b57b0c2, where we
accidentally started passing absolute offsets to the _boundary
parameter of op_get_next_page() instead of relative offsets.
We now use absolute offsets in all places, as it is the simpler
choice.
This matters now, because the error reported when encountering EOF
before hitting the _boundary is no longer suppressed (but instead
reported as OP_EBADLINK).
Finally, it removes the op_page_seek() function.
Except for the time needed to decode forward after seeking, this
function was identical in performance to op_pcm_seek(), and Opus
requires decoding 80 ms of data after seek anyway, so the relative
benefit is much smaller than with Vorbis.
A survey of open-source code using libvorbisfile showed that the
only usages of ov_page_seek() in the wild were calling it to seek
to the start of the stream, for which op_pcm_seek() already has a
special case that makes it just as fast.
The documentation was also updated to describe all of these chanes.
This is an incompatible API change.
--- a/examples/opusfile_example.c
+++ b/examples/opusfile_example.c
@@ -105,6 +105,7 @@
opus_int32 bitrate;
int ret;
int prev_li;
+ int is_ssl;
#if defined(_WIN32)
# undef fileno
# define fileno _fileno
@@ -120,6 +121,7 @@
fprintf(stderr,"Usage: %s <file.opus>\n",_argv[0]);
return EXIT_FAILURE;
}
+ is_ssl=0;
if(strcmp(_argv[1],"-")==0){ OpusFileCallbacks cb={NULL,NULL,NULL,NULL};of=op_open_callbacks(op_fdopen(&cb,fileno(stdin),"rb"),&cb,NULL,0,&ret);
@@ -139,6 +141,9 @@
}
#else
if(of==NULL)of=op_open_file(_argv[1],&ret);
+ /*This is not a very good check, but at least it won't give false
+ positives.*/
+ else is_ssl=strncmp(_argv[1],"https:",6)==0;
#endif
}
if(of==NULL){@@ -173,6 +178,7 @@
ret=op_read_native_stereo(of,pcm,sizeof(pcm)/sizeof(*pcm));
if(ret<0){fprintf(stderr,"\nError decoding '%s': %i\n",_argv[1],ret);
+ if(is_ssl)fprintf(stderr,"Possible truncation attack?\n");
ret=EXIT_FAILURE;
break;
}
--- a/examples/seeking_example.c
+++ b/examples/seeking_example.c
@@ -422,29 +422,6 @@
fprintf(stderr,"\rTotal seek operations: %li (%.3f per raw seek, %li maximum).\n",
nreal_seeks,nreal_seeks/(double)NSEEK_TESTS,max_seeks);
nreal_seeks=0;
- fprintf(stderr,"Testing PCM page seeking to random places in %li "
- "samples (",(long)pcm_length);- print_duration(stderr,pcm_length);
- fprintf(stderr,")...\n");
- max_seeks=0;
- for(i=0;i<NSEEK_TESTS;i++){- long nseeks_tmp;
- nseeks_tmp=nreal_seeks;
- pcm_offset=(ogg_int64_t)(rand()/(double)RAND_MAX*pcm_length);
- fprintf(stderr,"\r\t%3i [PCM position %li]... ",
- i,(long)pcm_offset);
- ret=op_pcm_seek_page(of,pcm_offset);
- if(ret<0){- fprintf(stderr,"\nSeek failed: %i.\n",ret);
- nfailures++;
- }
- verify_seek(of,-1,pcm_offset,pcm_length,bigassbuffer);
- nseeks_tmp=nreal_seeks-nseeks_tmp;
- max_seeks=nseeks_tmp>max_seeks?nseeks_tmp:max_seeks;
- }
- fprintf(stderr,"\rTotal seek operations: %li (%.3f per page seek, %li maximum).\n",
- nreal_seeks,nreal_seeks/(double)NSEEK_TESTS,max_seeks);
- nreal_seeks=0;
fprintf(stderr,"Testing exact PCM seeking to random places in %li "
"samples (",(long)pcm_length);print_duration(stderr,pcm_length);
--- a/include/opusfile.h
+++ b/include/opusfile.h
@@ -499,19 +499,15 @@
typedef struct OpusFileCallbacks OpusFileCallbacks;
-/**Reads \a _nmemb elements of data, each \a _size bytes long, from
- \a _stream.
- \return The number of items successfully read (i.e., not the number of
- characters).
- Unlike normal <code>fread()</code>, this function is allowed to
- return fewer items than requested (e.g., if reading more would
- block), as long as <em>some</em> data is returned when no error
- occurs and EOF has not been reached.
- If an error occurs, or the end-of-file is reached, the return
- value is zero.
- <code>errno</code> need not be set.*/
-typedef size_t (*op_read_func)(void *_ptr,size_t _size,size_t _nmemb,
- void *_stream);
+/**Reads up to \a _nbytes bytes of data from \a _stream.
+ \param _stream The stream to read from.
+ \param[out] _ptr The buffer to store the data in.
+ \param _nbytes The maximum number of bytes to read.
+ This function may return fewer, though it will not
+ return zero unless it reaches end-of-file.
+ \return The number of bytes successfully read, or a negative value on
+ error.*/
+typedef int (*op_read_func)(void *_stream,unsigned char *_ptr,int _nbytes);
/**Sets the position indicator for \a _stream.
The new position, measured in bytes, is obtained by adding \a _offset
@@ -731,8 +727,6 @@
size_t _size,int *_error);
/**Open a stream from a URL.
- See the security warning in op_open_url_with_proxy() for information about
- possible truncation attacks with HTTPS.
This function behaves identically to op_open_url(), except that it
takes a va_list instead of a variable number of arguments.
It does not call the <code>va_end</code> macro, and because it invokes the
@@ -757,16 +751,6 @@
int *_error,va_list _ap) OP_ARG_NONNULL(1);
/**Open a stream from a URL.
- \warning HTTPS streams that are not served with a Content-Length header may
- be vulnerable to truncation attacks.
- The abstract stream interface is incapable of signaling whether a connection
- was closed gracefully (with a TLS "close notify" message) or abruptly (and,
- e.g., possibly by an attacker).
- If you wish to guarantee that you are not vulnerable to such attacks, you
- might consider only allowing seekable streams (which must have a valid
- content length) and verifying the file position reported by op_raw_tell()
- after decoding to the end is at least as large as that reported by
- op_raw_total() (though possibly larger).
However, this approach will not work for live streams or HTTP/1.0 servers
(which do not support Range requets).
\param _url The URL to open.
@@ -1272,7 +1256,7 @@
\param _of The \c OggOpusFile in which to seek.
\param _byte_offset The byte position to seek to.
\return 0 on success, or a negative error code on failure.
- \retval #OP_EREAD The seek failed.
+ \retval #OP_EREAD The underlying seek operation failed.
\retval #OP_EINVAL The stream was only partially open, or the target was
outside the valid range for the stream.
\retval #OP_ENOSEEK This stream is not seekable.
@@ -1280,22 +1264,6 @@
unknown reason.*/
int op_raw_seek(OggOpusFile *_of,opus_int64 _byte_offset) OP_ARG_NONNULL(1);
-/**Seek to a page preceding the specified PCM offset, such that decoding will
- quickly arrive at the requested position.
- This is faster than sample-granularity seeking because it doesn't do the
- last bit of decode to find a specific sample.
- \param _of The \c OggOpusFile in which to seek.
- \param _pcm_offset The PCM offset to seek to.
- This is in samples at 48 kHz relative to the start of the
- stream.
- \return 0 on success, or a negative value on error.
- \retval #OP_EREAD The seek failed.
- \retval #OP_EINVAL The stream was only partially open, or the target was
- outside the valid range for the stream.
- \retval #OP_ENOSEEK This stream is not seekable.*/
-int op_pcm_seek_page(OggOpusFile *_of,ogg_int64_t _pcm_offset)
- OP_ARG_NONNULL(1);
-
/**Seek to the specified PCM offset, such that decoding will begin at exactly
the requested position.
\param _of The \c OggOpusFile in which to seek.
@@ -1303,10 +1271,13 @@
This is in samples at 48 kHz relative to the start of the
stream.
\return 0 on success, or a negative value on error.
- \retval #OP_EREAD The seek failed.
- \retval #OP_EINVAL The stream was only partially open, or the target was
- outside the valid range for the stream.
- \retval #OP_ENOSEEK This stream is not seekable.*/
+ \retval #OP_EREAD An underlying read or seek operation failed.
+ \retval #OP_EINVAL The stream was only partially open, or the target was
+ outside the valid range for the stream.
+ \retval #OP_ENOSEEK This stream is not seekable.
+ \retval #OP_EBADLINK We failed to find data we had seen before, or the
+ bitstream structure was sufficiently malformed that
+ seeking to the target destination was impossible.*/
int op_pcm_seek(OggOpusFile *_of,ogg_int64_t _pcm_offset) OP_ARG_NONNULL(1);
/*@}*/
@@ -1339,7 +1310,17 @@
If so, the floating-point API may also be disabled.
In that configuration, nothing in <tt>libopusfile</tt> will use any
floating-point operations, to simplify support on devices without an
- adequate FPU.*/
+ adequate FPU.
+
+ \warning HTTPS streams may be be vulnerable to truncation attacks if you do
+ not check the error return code from op_read_float() or its associated
+ functions.
+ If the remote peer does not close the connection gracefully (with a TLS
+ "close notify" message), these functions will return OP_EREAD instead of 0
+ when they reach the end of the file.
+ If you are reading from an <https:> URL (particularly if seeking is not
+ supported), you should make sure to check for this error and warn the user
+ appropriately.*/
/*@{*//**Reads more samples from the stream.
@@ -1396,6 +1377,13 @@
The list of possible failure codes follows.
Most of them can only be returned by unseekable, chained streams
that encounter a new link.
+ \retval #OP_HOLE There was a hole in the data, and some samples
+ may have been skipped.
+ Call this function again to continue decoding
+ past the hole.
+ \retval #OP_EREAD An underlying read operation failed.
+ This may signal a truncation attack from an
+ <https:> source.
\retval #OP_EFAULT An internal memory allocation failed.
\retval #OP_EIMPL An unseekable stream encountered a new link that
used a feature that is not implemented, such as
@@ -1411,6 +1399,7 @@
an ID header that contained an unrecognized
version number.
\retval #OP_EBADPACKET Failed to properly decode the next packet.
+ \retval #OP_EBADLINK We failed to find data we had seen before.
\retval #OP_EBADTIMESTAMP An unseekable stream encountered a new link with
a starting timestamp that failed basic validity
checks.*/
@@ -1469,6 +1458,13 @@
The list of possible failure codes follows.
Most of them can only be returned by unseekable, chained streams
that encounter a new link.
+ \retval #OP_HOLE There was a hole in the data, and some samples
+ may have been skipped.
+ Call this function again to continue decoding
+ past the hole.
+ \retval #OP_EREAD An underlying read operation failed.
+ This may signal a truncation attack from an
+ <https:> source.
\retval #OP_EFAULT An internal memory allocation failed.
\retval #OP_EIMPL An unseekable stream encountered a new link that
used a feature that is not implemented, such as
@@ -1484,6 +1480,7 @@
an ID header that contained an unrecognized
version number.
\retval #OP_EBADPACKET Failed to properly decode the next packet.
+ \retval #OP_EBADLINK We failed to find data we had seen before.
\retval #OP_EBADTIMESTAMP An unseekable stream encountered a new link with
a starting timestamp that failed basic validity
checks.*/
@@ -1522,6 +1519,13 @@
The list of possible failure codes follows.
Most of them can only be returned by unseekable, chained streams
that encounter a new link.
+ \retval #OP_HOLE There was a hole in the data, and some samples
+ may have been skipped.
+ Call this function again to continue decoding
+ past the hole.
+ \retval #OP_EREAD An underlying read operation failed.
+ This may signal a truncation attack from an
+ <https:> source.
\retval #OP_EFAULT An internal memory allocation failed.
\retval #OP_EIMPL An unseekable stream encountered a new link that
used a feature that is not implemented, such as
@@ -1537,6 +1541,7 @@
an ID header that contained an unrecognized
version number.
\retval #OP_EBADPACKET Failed to properly decode the next packet.
+ \retval #OP_EBADLINK We failed to find data we had seen before.
\retval #OP_EBADTIMESTAMP An unseekable stream encountered a new link with
a starting timestamp that failed basic validity
checks.*/
@@ -1575,6 +1580,13 @@
The list of possible failure codes follows.
Most of them can only be returned by unseekable, chained streams
that encounter a new link.
+ \retval #OP_HOLE There was a hole in the data, and some samples
+ may have been skipped.
+ Call this function again to continue decoding
+ past the hole.
+ \retval #OP_EREAD An underlying read operation failed.
+ This may signal a truncation attack from an
+ <https:> source.
\retval #OP_EFAULT An internal memory allocation failed.
\retval #OP_EIMPL An unseekable stream encountered a new link that
used a feature that is not implemented, such as
@@ -1590,6 +1602,7 @@
an ID header that contained an unrecognized
version number.
\retval #OP_EBADPACKET Failed to properly decode the next packet.
+ \retval #OP_EBADLINK We failed to find data we had seen before.
\retval #OP_EBADTIMESTAMP An unseekable stream encountered a new link with
a starting timestamp that failed basic validity
checks.*/
--- a/src/http.c
+++ b/src/http.c
@@ -766,20 +766,20 @@
}
static int op_http_conn_write_fully(OpusHTTPConn *_conn,
- const char *_buf,int _size){+ const char *_buf,int _buf_size){struct pollfd fd;
SSL *ssl_conn;
fd.fd=_conn->fd;
ssl_conn=_conn->ssl_conn;
- while(_size>0){+ while(_buf_size>0){int err;
if(ssl_conn!=NULL){int ret;
- ret=SSL_write(ssl_conn,_buf,_size);
+ ret=SSL_write(ssl_conn,_buf,_buf_size);
if(ret>0){/*Wrote some data.*/
_buf+=ret;
- _size-=ret;
+ _buf_size-=ret;
continue;
}
/*Connection closed.*/
@@ -793,10 +793,10 @@
else{ssize_t ret;
errno=0;
- ret=write(fd.fd,_buf,_size);
+ ret=write(fd.fd,_buf,_buf_size);
if(ret>0){_buf+=ret;
- _size-=ret;
+ _buf_size-=ret;
continue;
}
err=errno;
@@ -859,14 +859,17 @@
/*Tries to read from the given connection.
[out] _buf: Returns the data read.
- _size: The size of the buffer.
- _blocking: Whether or not to block until some data is retrieved.*/
-static ptrdiff_t op_http_conn_read(OpusHTTPConn *_conn,
- char *_buf,ptrdiff_t _size,int _blocking){- struct pollfd fd;
- SSL *ssl_conn;
- ptrdiff_t nread;
- ptrdiff_t nread_unblocked;
+ _buf_size: The size of the buffer.
+ _blocking: Whether or not to block until some data is retrieved.
+ Return: A positive number of bytes read on success.
+ 0: The read would block, or the connection was closed.
+ OP_EREAD: There was a fatal read error.*/
+static int op_http_conn_read(OpusHTTPConn *_conn,
+ unsigned char *_buf,int _buf_size,int _blocking){+ struct pollfd fd;
+ SSL *ssl_conn;
+ int nread;
+ int nread_unblocked;
fd.fd=_conn->fd;
ssl_conn=_conn->ssl_conn;
nread=nread_unblocked=0;
@@ -874,7 +877,8 @@
int err;
if(ssl_conn!=NULL){int ret;
- ret=SSL_read(ssl_conn,_buf+nread,_size-nread);
+ ret=SSL_read(ssl_conn,_buf+nread,_buf_size-nread);
+ OP_ASSERT(ret<=_buf_size-nread);
if(ret>0){/*Read some data.
Keep going to see if there's more.*/
@@ -882,20 +886,27 @@
nread_unblocked+=ret;
continue;
}
- /*Connection closed.*/
- else if(ret==0)break;
/*If we already read some data, return it right now.*/
if(nread>0)break;
err=SSL_get_error(ssl_conn,ret);
+ if(ret==0){+ /*Connection close.
+ Check for a clean shutdown to prevent truncation attacks.
+ This check always succeeds for SSLv2, as it has no "close notify"
+ message and thus can't verify an orderly shutdown.*/
+ return err==SSL_ERROR_ZERO_RETURN?0:OP_EREAD;
+ }
if(err==SSL_ERROR_WANT_READ)fd.events=POLLIN;
/*Yes, renegotiations can cause SSL_read() to block for writing.*/
else if(err==SSL_ERROR_WANT_WRITE)fd.events=POLLOUT;
- else return 0;
+ /*Some other error.*/
+ else return OP_EREAD;
}
else{ssize_t ret;
errno=0;
- ret=read(fd.fd,_buf+nread,_size-nread);
+ ret=read(fd.fd,_buf+nread,_buf_size-nread);
+ OP_ASSERT(ret<=_buf_size-nread);
if(ret>0){/*Read some data.
Keep going to see if there's more.*/
@@ -907,7 +918,7 @@
right now.*/
if(ret==0||nread>0)break;
err=errno;
- if(err!=EAGAIN&&err!=EWOULDBLOCK)return 0;
+ if(err!=EAGAIN&&err!=EWOULDBLOCK)return OP_EREAD;
fd.events=POLLIN;
}
_conn->read_bytes+=nread_unblocked;
@@ -915,9 +926,9 @@
nread_unblocked=0;
if(!_blocking)break;
/*Need to wait to get any data at all.*/
- if(poll(&fd,1,OP_POLL_TIMEOUT_MS)<=0)return 0;
+ if(poll(&fd,1,OP_POLL_TIMEOUT_MS)<=0)return OP_EREAD;
}
- while(nread<_size);
+ while(nread<_buf_size);
_conn->read_bytes+=nread_unblocked;
return nread;
}
@@ -924,9 +935,9 @@
/*Tries to look at the pending data for a connection without consuming it.
[out] _buf: Returns the data at which we're peeking.
- _size: The size of the buffer.*/
+ _buf_size: The size of the buffer.*/
static int op_http_conn_peek(OpusHTTPConn *_conn,
- char *_buf,int _size){+ char *_buf,int _buf_size){struct pollfd fd;
SSL *ssl_conn;
int ret;
@@ -935,7 +946,7 @@
for(;;){int err;
if(ssl_conn!=NULL){- ret=SSL_peek(ssl_conn,_buf,_size);
+ ret=SSL_peek(ssl_conn,_buf,_buf_size);
/*Either saw some data or the connection was closed.*/
if(ret>=0)return ret;
err=SSL_get_error(ssl_conn,ret);
@@ -946,7 +957,7 @@
}
else{errno=0;
- ret=(int)recv(fd.fd,_buf,_size,MSG_PEEK);
+ ret=(int)recv(fd.fd,_buf,_buf_size,MSG_PEEK);
/*Either saw some data or the connection was closed.*/
if(ret>=0)return ret;
err=errno;
@@ -1020,7 +1031,7 @@
OP_ASSERT(size<=read_limit);
OP_ASSERT(read_limit<=size+ret);
/*Actually consume that data.*/
- ret=op_http_conn_read(_conn,buf+size,read_limit-size,1);
+ ret=op_http_conn_read(_conn,(unsigned char *)buf+size,read_limit-size,1);
if(OP_UNLIKELY(ret<=0))return OP_FALSE;
size+=ret;
buf[size]='\0';
@@ -2267,15 +2278,17 @@
If we've reached the end of this response body, parse the next response and
keep going.
[out] _buf: Returns the data read.
- _size: The size of the buffer.
- _blocking: Whether or not to block until some data is retrieved.*/
-static ptrdiff_t op_http_conn_read_body(OpusHTTPStream *_stream,
- OpusHTTPConn *_conn,char *_buf,ptrdiff_t _size,int _blocking){+ _buf_size: The size of the buffer.
+ Return: A positive number of bytes read on success.
+ 0: The connection was closed.
+ OP_EREAD: There was a fatal read error.*/
+static int op_http_conn_read_body(OpusHTTPStream *_stream,
+ OpusHTTPConn *_conn,unsigned char *_buf,int _buf_size){opus_int64 pos;
opus_int64 end_pos;
opus_int64 next_pos;
opus_int64 content_length;
- ptrdiff_t nread;
+ int nread;
int pipeline;
int ret;
/*Currently this function can only be called on the LRU head.
@@ -2282,8 +2295,8 @@
Otherwise, we'd need a _pnext pointer if we needed to close the connection,
and re-opening it would re-organize the lists.*/
OP_ASSERT(_stream->lru_head==_conn);
- /*If we try an empty read, we won't be able to tell if we hit an error.*/
- OP_ASSERT(_size>0);
+ /*We should have filterd out empty reads by this point.*/
+ OP_ASSERT(_buf_size>0);
pos=_conn->pos;
end_pos=_conn->end_pos;
next_pos=_conn->next_pos;
@@ -2297,7 +2310,7 @@
Also return early if a non-blocking read was requested (regardless of
whether we might be able to parse the next response without
blocking).*/
- if(content_length<=end_pos||!_blocking)return 0;
+ if(content_length<=end_pos)return 0;
/*Otherwise, start on the next response.*/
if(next_pos<0){/*We haven't issued another request yet.*/
@@ -2315,12 +2328,12 @@
/*If we're not pipelining, we should be requesting the rest.*/
OP_ASSERT(pipeline||_conn->chunk_size==-1);
ret=op_http_conn_open_pos(_stream,_conn,end_pos,_conn->chunk_size);
- if(OP_UNLIKELY(ret<0))return 0;
+ if(OP_UNLIKELY(ret<0))return OP_EREAD;
}
else{/*Issue the request now (better late than never).*/
ret=op_http_conn_send_request(_stream,_conn,pos,_conn->chunk_size,0);
- if(OP_UNLIKELY(ret<0))return 0;
+ if(OP_UNLIKELY(ret<0))return OP_EREAD;
next_pos=_conn->next_pos;
OP_ASSERT(next_pos>=0);
}
@@ -2330,7 +2343,7 @@
seeking somewhere else.*/
OP_ASSERT(next_pos==end_pos);
ret=op_http_conn_handle_response(_stream,_conn);
- if(OP_UNLIKELY(ret<0))return 0;
+ if(OP_UNLIKELY(ret<0))return OP_EREAD;
if(OP_UNLIKELY(ret>0)&&pipeline){opus_int64 next_end;
next_end=_conn->next_end;
@@ -2343,9 +2356,9 @@
||next_end-next_pos>=0&&next_end-next_pos<=0x7FFFFFFF);
ret=op_http_conn_open_pos(_stream,_conn,next_pos,
next_end<0?-1:(opus_int32)(next_end-next_pos));
- if(OP_UNLIKELY(ret<0))return 0;
+ if(OP_UNLIKELY(ret<0))return OP_EREAD;
}
- else if(OP_UNLIKELY(ret!=0))return OP_FALSE;
+ else if(OP_UNLIKELY(ret!=0))return OP_EREAD;
}
pos=_conn->pos;
end_pos=_conn->end_pos;
@@ -2352,9 +2365,10 @@
content_length=_stream->content_length;
}
OP_ASSERT(end_pos>pos);
- _size=OP_MIN(_size,end_pos-pos);
+ _buf_size=OP_MIN(_buf_size,end_pos-pos);
}
- nread=op_http_conn_read(_conn,_buf,_size,_blocking);
+ nread=op_http_conn_read(_conn,_buf,_buf_size,1);
+ if(OP_UNLIKELY(nread<0))return nread;
pos+=nread;
_conn->pos=pos;
OP_ASSERT(end_pos<0||content_length>=0);
@@ -2378,24 +2392,22 @@
if(chunk_size>=0)request_thresh=OP_MIN(chunk_size>>2,request_thresh);
if(end_pos-pos<=request_thresh){ret=op_http_conn_send_request(_stream,_conn,end_pos,_conn->chunk_size,1);
- if(OP_UNLIKELY(ret<0))return 0;
+ if(OP_UNLIKELY(ret<0))return OP_EREAD;
}
}
return nread;
}
-static size_t op_http_stream_read(void *_ptr,size_t _size,size_t _nmemb,
- void *_stream){+static int op_http_stream_read(void *_stream,
+ unsigned char *_ptr,int _buf_size){OpusHTTPStream *stream;
ptrdiff_t nread;
- ptrdiff_t total;
opus_int64 size;
opus_int64 pos;
int ci;
stream=(OpusHTTPStream *)_stream;
- total=_size*_nmemb;
- /*Check for overflow/empty read.*/
- if(total==0||total/_size!=_nmemb||total>OP_INT64_MAX)return 0;
+ /*Check for an empty read.*/
+ if(_buf_size<=0)return 0;
ci=stream->cur_conni;
/*No current connection => EOF.*/
if(ci<0)return 0;
@@ -2405,42 +2417,11 @@
if(size>=0){if(pos>=size)return 0;
/*Check for a short read.*/
- if(total>size-pos){- _nmemb=(size-pos)/_size;
- total=_size*_nmemb;
- }
+ if(_buf_size>size-pos)_buf_size=(int)(size-pos);
}
- if(_size==1){- nread=op_http_conn_read_body(stream,stream->conns+ci,_ptr,total,1);
- }
- else{- ptrdiff_t n;
- nread=0;
- /*libopusfile doesn't read multi-byte items, but our abstract stream API
- requires it for stdio compatibility.
- Implement it for completeness' sake by reading individual items one at a
- time.*/
- do{- ptrdiff_t nread_item;
- nread_item=0;
- do{- /*Block on the first item, or if we've gotten a partial item.*/
- n=op_http_conn_read_body(stream,stream->conns+ci,
- _ptr,_size-nread_item,nread==0||nread_item>0);
- nread_item+=n;
- }
- while(n>0&&nread_item<(ptrdiff_t)_size);
- /*We can still fail to read a whole item if we encounter an error, or if
- we hit EOF and didn't know the stream length.
- TODO: The former is okay, the latter is not, but I don't know how to
- fix it without buffering arbitrarily large amounts of data.*/
- if(nread_item>=(ptrdiff_t)_size)nread++;
- total-=_size;
- }
- while(n>0&&total>0);
- }
+ nread=op_http_conn_read_body(stream,stream->conns+ci,_ptr,_buf_size);
if(OP_UNLIKELY(nread<=0)){- /*We either hit an error or EOF.
+ /*We hit an error or EOF.
Either way, we're done with this connection.*/
op_http_conn_close(stream,stream->conns+ci,&stream->lru_head,1);
stream->cur_conni=-1;
@@ -2458,7 +2439,7 @@
_target: The stream position to which to read ahead.*/
static int op_http_conn_read_ahead(OpusHTTPStream *_stream,
OpusHTTPConn *_conn,int _just_read_ahead,opus_int64 _target){- static char dummy_buf[OP_READAHEAD_CHUNK_SIZE];
+ static unsigned char dummy_buf[OP_READAHEAD_CHUNK_SIZE];
opus_int64 pos;
opus_int64 end_pos;
opus_int64 next_pos;
@@ -2489,7 +2470,7 @@
Finish off the current chunk.*/
while(pos<end_pos){nread=op_http_conn_read(_conn,dummy_buf,
- OP_MIN(end_pos-pos,OP_READAHEAD_CHUNK_SIZE),1);
+ (int)OP_MIN(end_pos-pos,OP_READAHEAD_CHUNK_SIZE),1);
/*We failed to read ahead.*/
if(nread<=0)return OP_FALSE;
pos+=nread;
@@ -2514,7 +2495,7 @@
}
while(pos<end_pos){nread=op_http_conn_read(_conn,dummy_buf,
- OP_MIN(end_pos-pos,OP_READAHEAD_CHUNK_SIZE),1);
+ (int)OP_MIN(end_pos-pos,OP_READAHEAD_CHUNK_SIZE),1);
/*We failed to read ahead.*/
if(nread<=0)return OP_FALSE;
pos+=nread;
--- a/src/opusfile.c
+++ b/src/opusfile.c
@@ -134,18 +134,17 @@
/*The read/seek functions track absolute position within the stream.*/
-/*Read a little more data from the file/pipe into the ogg_sync framer.*/
+/*Read a little more data from the file/pipe into the ogg_sync framer.
+ Return: A positive number of bytes read on success, 0 on end-of-file, or a
+ negative value on failure.*/
static int op_get_data(OggOpusFile *_of){- char *buffer;
- int bytes;
- buffer=ogg_sync_buffer(&_of->oy,OP_READ_SIZE);
- bytes=(int)(*_of->callbacks.read)(buffer,
- 1,OP_READ_SIZE,_of->source);
- if(OP_LIKELY(bytes>0)){- ogg_sync_wrote(&_of->oy,bytes);
- return bytes;
- }
- return OP_EREAD;
+ unsigned char *buffer;
+ int bytes;
+ buffer=(unsigned char *)ogg_sync_buffer(&_of->oy,OP_READ_SIZE);
+ bytes=(int)(*_of->callbacks.read)(_of->source,buffer,OP_READ_SIZE);
+ OP_ASSERT(bytes<=OP_READ_SIZE);
+ if(OP_LIKELY(bytes>0))ogg_sync_wrote(&_of->oy,bytes);
+ return bytes;
}
/*Save a tiny smidge of verbosity to make the code more readable.*/
@@ -171,16 +170,16 @@
/*From the head of the stream, get the next page.
_boundary specifies if the function is allowed to fetch more data from the
stream (and how much) or only use internally buffered data.
- _boundary: -1) Unbounded search.
- 0) Read no additional data.
+ _boundary: -1: Unbounded search.
+ 0: Read no additional data.
Use only cached data.
- n) Search for the start of a new page for n bytes.
- Return: n>=0) Found a page at absolute offset n.
- OP_FALSE) Hit the _boundary limit.
- OP_EREAD) Failed to read more data.*/
+ n: Search for the start of a new page up to file position n.
+ Return: n>=0: Found a page at absolute offset n.
+ OP_FALSE: Hit the _boundary limit.
+ OP_EREAD: An underlying read operation failed.
+ OP_BADLINK: We hit end-of-file before reaching _boundary.*/
static opus_int64 op_get_next_page(OggOpusFile *_of,ogg_page *_og,
opus_int64 _boundary){- if(_boundary>0)_boundary+=_of->offset;
for(;;){int more;
if(_boundary>0&&_of->offset>=_boundary)return OP_FALSE;
@@ -192,14 +191,13 @@
/*Send more paramedics.*/
if(!_boundary)return OP_FALSE;
ret=op_get_data(_of);
- if(OP_UNLIKELY(ret<0)){- opus_int64 read_offset;
- /*If we read up to the boundary (or EOF in a seekable stream),
- including buffered sync data, then treat this as EOF.
- Otherwise treat it as a read error.*/
- if(_boundary<0)_boundary=_of->end;
- read_offset=op_position(_of);
- return read_offset>=_boundary?OP_FALSE:ret;
+ if(OP_UNLIKELY(ret<0))return OP_EREAD;
+ if(OP_UNLIKELY(ret==0)){+ /*If we encounter an EOF, return an error if we didn't at least read up
+ to the boundary (if known).
+ This test always succeeds if _boundary is -1, but that only happens
+ in unseekable streams.*/
+ return op_position(_of)>=_boundary?OP_FALSE:OP_EBADLINK;
}
}
else{@@ -311,7 +309,7 @@
while(_of->offset<end){opus_int64 llret;
ogg_uint32_t serialno;
- llret=op_get_next_page(_of,&og,end-_of->offset);
+ llret=op_get_next_page(_of,&og,end);
if(OP_UNLIKELY(llret<OP_FALSE))return (int)llret;
else if(llret==OP_FALSE)break;
serialno=ogg_page_serialno(&og);
@@ -391,8 +389,9 @@
_of->ready_state=OP_STREAMSET;
}
}
- /*Get the next page.*/
- llret=op_get_next_page(_of,_og,OP_CHUNK_SIZE);
+ /*Get the next page.
+ No need to clamp _boundary as all errors become OP_ENOTFORMAT.*/
+ llret=op_get_next_page(_of,_og,_of->offset+OP_CHUNK_SIZE);
if(OP_UNLIKELY(llret<0))return OP_ENOTFORMAT;
/*If this page also belongs to our Opus stream, submit it and break.*/
if(_of->ready_state==OP_STREAMSET
@@ -408,7 +407,9 @@
case 0:{/*Loop getting pages.*/
for(;;){- if(OP_UNLIKELY(op_get_next_page(_of,_og,OP_CHUNK_SIZE)<0)){+ /*No need to clamp _boundary as all errors become OP_EBADHEADER.*/
+ if(OP_UNLIKELY(op_get_next_page(_of,_og,
+ _of->offset+OP_CHUNK_SIZE)<0)){return OP_EBADHEADER;
}
/*If this page belongs to the correct stream, go parse it.*/
@@ -453,7 +454,8 @@
int ret;
if(!_og){ogg_int64_t llret;
- llret=op_get_next_page(_of,&og,OP_CHUNK_SIZE);
+ /*No need to clamp _boundary as all errors become OP_ENOTFORMAT.*/
+ llret=op_get_next_page(_of,&og,_of->offset+OP_CHUNK_SIZE);
if(OP_UNLIKELY(llret<0))return OP_ENOTFORMAT;
_og=&og;
}
@@ -715,9 +717,13 @@
least once.*/
total_duration=0;
do{+ opus_int64 llret;
+ llret=op_get_next_page(_of,_og,_of->end);
/*We should get a page unless the file is truncated or mangled.
Otherwise there are no audio data packets in the whole logical stream.*/
- if(OP_UNLIKELY(op_get_next_page(_of,_og,_of->end)<0)){+ if(OP_UNLIKELY(llret<0)){+ /*Fail if there was a read error.*/
+ if(llret<OP_FALSE)return (int)llret;
/*Fail if the pre-skip is non-zero, since it's asking us to skip more
samples than exist.*/
if(_link->head.pre_skip>0)return OP_EBADTIMESTAMP;
@@ -1070,7 +1076,7 @@
if(OP_UNLIKELY(ret<0))return ret;
last=op_get_next_page(_of,&og,_of->end);
/*At the worst we should have hit the page at _sr[sri-1].offset.*/
- if(OP_UNLIKELY(last<0))return OP_EBADLINK;
+ if(OP_UNLIKELY(last<0))return last<OP_FALSE?(int)last:OP_EBADLINK;
OP_ASSERT(nsr<_csr);
_sr[nsr].serialno=ogg_page_serialno(&og);
_sr[nsr].gp=ogg_page_granulepos(&og);
@@ -1703,7 +1709,7 @@
/*Keep reading until we get a page with the correct serialno.*/
else _page_pos=op_get_next_page(_of,&og,_of->end);
/*EOF: Leave uninitialized.*/
- if(_page_pos<0)return OP_EOF;
+ if(_page_pos<0)return _page_pos<OP_FALSE?(int)_page_pos:OP_EOF;
if(OP_LIKELY(_of->ready_state>=OP_STREAMSET)){ if(cur_serialno!=(ogg_uint32_t)ogg_page_serialno(&og)){/*Two possibilities:
@@ -2007,7 +2013,7 @@
There is a danger here: missing pages or incorrect frame number information
in the bitstream could make our task impossible.
Account for that (it would be an error condition).*/
-static int op_pcm_seek_page_impl(OggOpusFile *_of,
+static int op_pcm_seek_page(OggOpusFile *_of,
ogg_int64_t _target_gp,int _li){OggOpusLink *link;
ogg_page og;
@@ -2079,9 +2085,9 @@
}
chunk_size=OP_CHUNK_SIZE;
while(begin<end){- page_offset=op_get_next_page(_of,&og,end-_of->offset);
- if(page_offset==OP_EREAD)return OP_EBADLINK;
+ page_offset=op_get_next_page(_of,&og,end);
if(page_offset<0){+ if(page_offset<OP_FALSE)return (int)page_offset;
/*There are no more pages in our interval from our stream with a valid
timestamp that start at position bisect or later.*/
/*If we scanned the whole interval, we're done.*/
@@ -2179,17 +2185,6 @@
return 0;
}
-int op_pcm_seek_page(OggOpusFile *_of,ogg_int64_t _pcm_offset){- ogg_int64_t target_gp;
- int li;
- if(OP_UNLIKELY(_of->ready_state<OP_OPENED))return OP_EINVAL;
- if(OP_UNLIKELY(!_of->seekable))return OP_ENOSEEK;
- if(OP_UNLIKELY(_pcm_offset<0))return OP_EINVAL;
- target_gp=op_get_granulepos(_of,_pcm_offset,&li);
- if(OP_UNLIKELY(target_gp==-1))return OP_EINVAL;
- return op_pcm_seek_page_impl(_of,target_gp,li);
-}
-
int op_pcm_seek(OggOpusFile *_of,ogg_int64_t _pcm_offset){OggOpusLink *link;
ogg_int64_t pcm_start;
@@ -2206,7 +2201,7 @@
if(OP_UNLIKELY(_pcm_offset<0))return OP_EINVAL;
target_gp=op_get_granulepos(_of,_pcm_offset,&li);
if(OP_UNLIKELY(target_gp==-1))return OP_EINVAL;
- ret=op_pcm_seek_page_impl(_of,target_gp,li);
+ ret=op_pcm_seek_page(_of,target_gp,li);
/*Now skip samples until we actually get to our target.*/
link=_of->links+li;
pcm_start=link->pcm_start;
--- a/src/stream.c
+++ b/src/stream.c
@@ -41,6 +41,18 @@
ptrdiff_t pos;
};
+static int op_fread(void *_stream,unsigned char *_ptr,int _buf_size){+ FILE *stream;
+ size_t ret;
+ /*Check for empty read.*/
+ if(_buf_size<=0)return 0;
+ stream=(FILE *)_stream;
+ ret=fread(_ptr,1,_buf_size,stream);
+ OP_ASSERT(ret<=(size_t)_buf_size);
+ /*If ret==0 and !feof(stream), there was a read error.*/
+ return ret>0||feof(stream)?(int)ret:OP_EREAD;
+}
+
static int op_fseek(void *_stream,opus_int64 _offset,int _whence){#if defined(_MSC_VER)
return _fseeki64((FILE *)_stream,_offset,_whence);
@@ -58,7 +70,7 @@
}
static const OpusFileCallbacks OP_FILE_CALLBACKS={- (op_read_func)fread,
+ op_fread,
op_fseek,
op_ftell,
(op_close_func)fclose
@@ -86,29 +98,23 @@
return fp;
}
-static size_t op_mem_read(void *_ptr,size_t _size,size_t _nmemb,void *_stream){+static int op_mem_read(void *_stream,unsigned char *_ptr,int _buf_size){OpusMemStream *stream;
- size_t total;
ptrdiff_t size;
ptrdiff_t pos;
stream=(OpusMemStream *)_stream;
- if(_size>OP_MEM_SIZE_MAX)return 0;
- total=_size*_nmemb;
- /*Check for overflow/empty read.*/
- if(total==0||total/_size!=_nmemb)return 0;
+ /*Check for empty read.*/
+ if(_buf_size<=0)return 0;
size=stream->size;
pos=stream->pos;
/*Check for EOF.*/
if(pos>=size)return 0;
/*Check for a short read.*/
- if(total>(size_t)(size-pos)){- _nmemb=(size-pos)/_size;
- total=_size*_nmemb;
- }
- memcpy(_ptr,stream->data+pos,total);
- pos+=total;
+ _buf_size=(int)OP_MAX(size-pos,_buf_size);
+ memcpy(_ptr,stream->data+pos,_buf_size);
+ pos+=_buf_size;
stream->pos=pos;
- return _nmemb;
+ return _buf_size;
}
static int op_mem_seek(void *_stream,opus_int64 _offset,int _whence){