You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by hw...@apache.org on 2010/10/28 17:28:34 UTC
svn commit: r1028352 - in /subversion/trunk: ./
subversion/libsvn_ra_svn/marshal.c
Author: hwright
Date: Thu Oct 28 15:28:33 2010
New Revision: 1028352
URL: http://svn.apache.org/viewvc?rev=1028352&view=rev
Log:
Merge r985606, r1028092 from the performance branch.
This change preallocates some svnserve buffers (but not ones so large as to
replicate CAN-2004-0413).
Modified:
subversion/trunk/ (props changed)
subversion/trunk/subversion/libsvn_ra_svn/marshal.c
Propchange: subversion/trunk/
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Thu Oct 28 15:28:33 2010
@@ -23,7 +23,7 @@
/subversion/branches/log-g-performance:870941-871032
/subversion/branches/merge-skips-obstructions:874525-874615
/subversion/branches/nfc-nfd-aware-client:870276,870376
-/subversion/branches/performance:982355,983764,983766,984927,984984,985014,985037,985046,985472,985477,985482,985500,985669,987888,987893,995507,995603,1001413,1025660
+/subversion/branches/performance:982355,983764,983766,984927,984984,985014,985037,985046,985472,985477,985482,985500,985606,985669,987888,987893,995507,995603,1001413,1025660,1028092
/subversion/branches/ra_serf-digest-authn:875693-876404
/subversion/branches/reintegrate-improvements:873853-874164
/subversion/branches/subtree-mergeinfo:876734-878766
Modified: subversion/trunk/subversion/libsvn_ra_svn/marshal.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_svn/marshal.c?rev=1028352&r1=1028351&r2=1028352&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_svn/marshal.c (original)
+++ subversion/trunk/subversion/libsvn_ra_svn/marshal.c Thu Oct 28 15:28:33 2010
@@ -44,6 +44,12 @@
#define svn_iswhitespace(c) ((c) == ' ' || (c) == '\n')
+/* If we receive data that *claims* to be followed by a very long string,
+ * we should not trust that claim right away. But everything up to 1 MB
+ * should be too small to be instrumental for a DOS attack. */
+
+#define SUSPICIOUSLY_HUGE_STRING_SIZE_THRESHOLD (0x100000)
+
/* --- CONNECTION INITIALIZATION --- */
svn_ra_svn_conn_t *svn_ra_svn_create_conn(apr_socket_t *sock,
@@ -296,8 +302,8 @@ static svn_error_t *readbuf_fill(svn_ra_
return SVN_NO_ERROR;
}
-static svn_error_t *readbuf_getchar(svn_ra_svn_conn_t *conn, apr_pool_t *pool,
- char *result)
+static APR_INLINE svn_error_t *
+readbuf_getchar(svn_ra_svn_conn_t *conn, apr_pool_t *pool, char *result)
{
if (conn->read_ptr == conn->read_end)
SVN_ERR(readbuf_fill(conn, pool));
@@ -537,15 +543,14 @@ svn_error_t *svn_ra_svn_write_tuple(svn_
/* --- READING DATA ITEMS --- */
-/* 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)
+/* 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)
{
char readbuf[4096];
apr_size_t readbuf_len;
- svn_stringbuf_t *stringbuf = svn_stringbuf_create("", pool);
/* We can't store strings longer than the maximum size of apr_size_t,
* so check for wrapping */
@@ -565,6 +570,57 @@ static svn_error_t *read_string(svn_ra_s
len -= readbuf_len;
}
+ return SVN_NO_ERROR;
+}
+
+/* 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;
+
+ /* 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(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;
+ }
+ }
+
+ /* Return the string properly wrapped into an RA_SVN item.
+ */
item->kind = SVN_RA_SVN_STRING;
item->u.string = apr_palloc(pool, sizeof(*item->u.string));
item->u.string->data = stringbuf->data;
@@ -625,7 +681,8 @@ static svn_error_t *read_item(svn_ra_svn
else if (svn_ctype_isalpha(c))
{
/* It's a word. */
- str = svn_stringbuf_ncreate(&c, 1, pool);
+ str = svn_stringbuf_create_ensure(16, pool);
+ svn_stringbuf_appendbyte(str, c);
while (1)
{
SVN_ERR(readbuf_getchar(conn, pool, &c));
@@ -640,7 +697,7 @@ static svn_error_t *read_item(svn_ra_svn
{
/* Read in the list items. */
item->kind = SVN_RA_SVN_LIST;
- item->u.list = apr_array_make(pool, 0, sizeof(svn_ra_svn_item_t));
+ item->u.list = apr_array_make(pool, 4, sizeof(svn_ra_svn_item_t));
while (1)
{
SVN_ERR(readbuf_getchar_skip_whitespace(conn, pool, &c));