Author: stefan2
Date: Mon Jun 13 18:22:42 2011
New Revision: 1135210
URL: http://svn.apache.org/viewvc?rev=1135210&view=rev
Log:
Fuse read_long_string and read_string functionality in an attempt to
increase test code overage. Keep the overhead small for short strings.
* subversion/libsvn_ra_svn/marshal.c
(read_long_string): drop
(read_string): handle 32 bit overflows; limit initial string buffer size
Patch by: julianfoad
(with modifications by me)
Modified:
subversion/trunk/subversion/libsvn_ra_svn/marshal.c
Modified: subversion/trunk/subversion/libsvn_ra_svn/marshal.c
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_svn/marshal.c?rev=1135210&r1=1135209&r2=1135210&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_svn/marshal.c (original)
+++ subversion/trunk/subversion/libsvn_ra_svn/marshal.c Mon Jun 13 18:22:42 2011
@@ -560,82 +560,65 @@ svn_error_t *svn_ra_svn_write_tuple(svn_
/* --- READING DATA ITEMS --- */
-/* Read LEN bytes from CONN into a supposedly empty STRINGBUF.
- * POOL will be used for temporary allocations. */
-static svn_error_t *
-read_long_string(svn_ra_svn_conn_t *conn, apr_pool_t *pool,
- svn_stringbuf_t *stringbuf, apr_uint64_t len)
+/* Read LEN bytes from CONN into already-allocated structure ITEM.
+ * Afterwards, *ITEM is of type 'SVN_RA_SVN_STRING', and its string
+ * data is allocated in POOL. */
+static svn_error_t *read_string(svn_ra_svn_conn_t *conn, apr_pool_t *pool,
+ svn_ra_svn_item_t *item, apr_uint64_t len64)
{
- char readbuf[4096];
+ svn_stringbuf_t *stringbuf;
+ apr_size_t len = (apr_size_t)len64;
apr_size_t readbuf_len;
+ char *dest;
/* We can't store strings longer than the maximum size of apr_size_t,
* so check for wrapping */
- if (((apr_size_t) len) < len)
+ if (len64 > APR_SIZE_MAX)
return svn_error_create(SVN_ERR_RA_SVN_MALFORMED_DATA, NULL,
_("String length larger than maximum"));
- while (len)
+ /* Read the string in chunks. The chunk size is large enough to avoid
+ * re-allocation in typical cases, and small enough to ensure we do not
+ * pre-allocate an unreasonable amount of memory if (perhaps due to
+ * network data corruption or a DOS attack), we receive a bogus claim that
+ * a very long string is going to follow. In that case, we start small
+ * and wait for all that data to actually show up. This does not fully
+ * prevent DOS attacks but makes them harder (you have to actually send
+ * gigabytes of data). */
+ readbuf_len = len < SUSPICIOUSLY_HUGE_STRING_SIZE_THRESHOLD
+ ? len
+ : SUSPICIOUSLY_HUGE_STRING_SIZE_THRESHOLD;
+ stringbuf = svn_stringbuf_create_ensure(readbuf_len, pool);
+ dest = stringbuf->data;
+
+ /* Read remaining string data directly into the string structure.
+ * Do it iteratively, if necessary. */
+ while (readbuf_len)
{
- readbuf_len = len > sizeof(readbuf) ? sizeof(readbuf) : (apr_size_t)len;
+ SVN_ERR(readbuf_read(conn, pool, dest, readbuf_len));
- SVN_ERR(readbuf_read(conn, pool, readbuf, readbuf_len));
- /* Read into a stringbuf_t to so we don't allow the sender to allocate
- * an arbitrary amount of memory without actually sending us that much
- * data */
- svn_stringbuf_appendbytes(stringbuf, readbuf, readbuf_len);
+ stringbuf->len += readbuf_len;
len -= readbuf_len;
- }
- return SVN_NO_ERROR;
-}
+ /* Early exit. In most cases, strings can be read in the first
+ * iteration. */
+ if (len == 0)
+ break;
-/* Read LEN bytes from CONN into already-allocated structure ITEM.
- * Afterwards, *ITEM is of type 'SVN_RA_SVN_STRING', and its string
- * data is allocated in POOL. */
-static svn_error_t *read_string(svn_ra_svn_conn_t *conn, apr_pool_t *pool,
- svn_ra_svn_item_t *item, apr_uint64_t len)
-{
- svn_stringbuf_t *stringbuf;
+ /* Prepare next iteration: determine length of chunk to read
+ * and re-alloc the string buffer. */
+ readbuf_len
+ = len < SUSPICIOUSLY_HUGE_STRING_SIZE_THRESHOLD
+ ? len
+ : SUSPICIOUSLY_HUGE_STRING_SIZE_THRESHOLD;
- /* We should not use large strings in our protocol. However, we may
- * receive a claim that a very long string is going to follow. In that
- * case, we start small and wait for all that data to actually show up.
- * This does not fully prevent DOS attacs but makes them harder (you
- * have to actually send gigabytes of data).
- */
- if (len > SUSPICIOUSLY_HUGE_STRING_SIZE_THRESHOLD)
- {
- /* This string might take a large amount of memory. Don't allocate
- * the whole buffer at once, so to prevent OOM issues by corrupted
- * network data.
- */
- stringbuf = svn_stringbuf_create("", pool);
- SVN_ERR(read_long_string(conn, pool, stringbuf, len));
- }
- else
- {
- /* This is a reasonably sized string. So, provide a buffer large
- * enough to prevent re-allocation as long as the data transmission
- * is not flawed.
- */
- stringbuf = svn_stringbuf_create_ensure((apr_size_t)len, pool);
-
- /* Read the string data directly into the string structure.
- * Do it iteratively, if necessary.
- */
- while (len)
- {
- apr_size_t readbuf_len = (apr_size_t)len;
- char *dest = stringbuf->data + stringbuf->len;
- SVN_ERR(readbuf_read(conn, pool, dest, readbuf_len));
-
- stringbuf->len += readbuf_len;
- stringbuf->data[stringbuf->len] = '\0';
- len -= readbuf_len;
- }
+ svn_stringbuf_ensure(stringbuf, stringbuf->len + readbuf_len + 1);
+ dest = stringbuf->data + stringbuf->len;
}
+ /* zero-terminate the string */
+ stringbuf->data[stringbuf->len] = '\0';
+
/* Return the string properly wrapped into an RA_SVN item. */
item->kind = SVN_RA_SVN_STRING;
item->u.string = svn_stringbuf__morph_into_string(stringbuf);
