You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by st...@apache.org on 2015/11/14 15:09:19 UTC
svn commit: r1714330 - in /subversion/trunk/subversion:
include/svn_error_codes.h include/svn_ra_svn.h libsvn_ra_svn/deprecated.c
libsvn_ra_svn/editorp.c libsvn_ra_svn/marshal.c libsvn_ra_svn/ra_svn.h
svnserve/serve.c svnserve/server.h svnserve/svnserve.c
Author: stefan2
Date: Sat Nov 14 14:09:18 2015
New Revision: 1714330
URL: http://svn.apache.org/viewvc?rev=1714330&view=rev
Log:
Add the equivalent of LimitXMLRequestBody to svnserve.
The idea is simple, whenever we fill our receive buffer, we update the sum
total and compare it to some limit. Reset the counter sum at each new
command / request coming in.
If a client request exceeds the --max-request-size parameter given to
svnserve (16MB by default, twice the httpd default), the processing gets
terminated and the connection will be closed. The latter is necessary
because the protocol is stateful and we just skipped / ignored a potential
state transition.
As a result, the memory usage of a threaded server is now bound to approx.
(max-request-size + 4M) x max-threads even in high-load scenarios. On the
flip side, propsets are limited to around 15M per property by default.
* subversion/include/svn_error_codes.h
(SVN_ERR_RA_SVN_REQUEST_SIZE): New error code.
* subversion/include/svn_ra_svn.h
(svn_ra_svn_create_conn5): Bumped API, adding the new limit parameter.
(svn_ra_svn_create_conn4): Deprecate.
* subversion/libsvn_ra_svn/ra_svn.h
(svn_ra_svn_conn_st): Add fields for the data counter and its limit.
(svn_ra_svn__reset_command_io_counters): Declare a function to reset the
counter - to be called before
each new command.
* subversion/libsvn_ra_svn/deprecated.c
(svn_ra_svn_create_conn4): Implement in terms of the new API.
* subversion/libsvn_ra_svn/marshal.c
(svn_ra_svn_create_conn5): Implement like the predecessor but init the
new struct elements as well.
(svn_ra_svn__reset_command_io_counters): Implement new private API.
(check_io_limits): New function performing the actual error detection.
(readbuf_input): Count incoming data and enforce limits.
(svn_ra_svn__has_command): Be sure to count I/O per command.
(svn_ra_svn__handle_command): Same. Also handle the case that we truncated
I/O and are now in a potentially inconsistent
state.
* subversion/libsvn_ra_svn/editorp.c
(svn_ra_svn_drive_editor2): Limit the request size separately for each
editor command - not the whole editor drive.
* subversion/svnserve/server.h
(serve_params_t): Add field for the new --max-request-size option.
* subversion/svnserve/serve.c
(serve_interruptable): Pass the new option to the bumped API.
* subversion/svnserve/svnserve.c
(MAX_REQUEST_SIZE): Define the default value for the new option.
(SVNSERVE_OPT_MAX_REQUEST): Declare the new option.
(svnserve__options): Define and document the new option.
(sub_main): Handle the new option and pass it to the bumped API.
Modified:
subversion/trunk/subversion/include/svn_error_codes.h
subversion/trunk/subversion/include/svn_ra_svn.h
subversion/trunk/subversion/libsvn_ra_svn/deprecated.c
subversion/trunk/subversion/libsvn_ra_svn/editorp.c
subversion/trunk/subversion/libsvn_ra_svn/marshal.c
subversion/trunk/subversion/libsvn_ra_svn/ra_svn.h
subversion/trunk/subversion/svnserve/serve.c
subversion/trunk/subversion/svnserve/server.h
subversion/trunk/subversion/svnserve/svnserve.c
Modified: subversion/trunk/subversion/include/svn_error_codes.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_error_codes.h?rev=1714330&r1=1714329&r2=1714330&view=diff
==============================================================================
--- subversion/trunk/subversion/include/svn_error_codes.h (original)
+++ subversion/trunk/subversion/include/svn_error_codes.h Sat Nov 14 14:09:18 2015
@@ -1542,6 +1542,11 @@ SVN_ERROR_START
SVN_ERR_RA_SVN_CATEGORY_START + 8,
"Editor drive was aborted")
+ /** @since New in 1.10 */
+ SVN_ERRDEF(SVN_ERR_RA_SVN_REQUEST_SIZE,
+ SVN_ERR_RA_SVN_CATEGORY_START + 9,
+ "Client request too long")
+
/* libsvn_auth errors */
/* this error can be used when an auth provider doesn't have
Modified: subversion/trunk/subversion/include/svn_ra_svn.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_ra_svn.h?rev=1714330&r1=1714329&r2=1714330&view=diff
==============================================================================
--- subversion/trunk/subversion/include/svn_ra_svn.h (original)
+++ subversion/trunk/subversion/include/svn_ra_svn.h Sat Nov 14 14:09:18 2015
@@ -189,13 +189,34 @@ typedef svn_error_t *(*svn_ra_svn_edit_c
* It defines the number of bytes that must have been sent since the last
* check before the next check will be made.
*
+ * If @a max_in is not 0, error out and close the connection whenever more
+ * than @a max_in bytes are received for a command (e.g. a client request).
+ * The limit enforced may be slightly different, +/- the I/O buffer size.
+ *
* @note If @a out_stream is an wrapped apr_file_t* the backing file will be
* used for some operations.
*
* Allocate the result in @a pool.
*
+ * @since New in 1.10
+ */
+svn_ra_svn_conn_t *svn_ra_svn_create_conn5(apr_socket_t *sock,
+ svn_stream_t *in_stream,
+ svn_stream_t *out_stream,
+ int compression_level,
+ apr_size_t zero_copy_limit,
+ apr_size_t error_check_interval,
+ apr_uint64_t max_in,
+ apr_pool_t *result_pool);
+
+
+/** Similar to svn_ra_svn_create_conn5() but with @a max_in and @a max_out
+ * set to 0.
+ *
* @since New in 1.9
+ * @deprecated Provided for backward compatibility with the 1.9 API.
*/
+SVN_DEPRECATED
svn_ra_svn_conn_t *svn_ra_svn_create_conn4(apr_socket_t *sock,
svn_stream_t *in_stream,
svn_stream_t *out_stream,
Modified: subversion/trunk/subversion/libsvn_ra_svn/deprecated.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_svn/deprecated.c?rev=1714330&r1=1714329&r2=1714330&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_svn/deprecated.c (original)
+++ subversion/trunk/subversion/libsvn_ra_svn/deprecated.c Sat Nov 14 14:09:18 2015
@@ -262,6 +262,20 @@ svn_ra_svn_write_cmd_failure(svn_ra_svn_
/* From marshal.c */
svn_ra_svn_conn_t *
+svn_ra_svn_create_conn4(apr_socket_t *sock,
+ svn_stream_t *in_stream,
+ svn_stream_t *out_stream,
+ int compression_level,
+ apr_size_t zero_copy_limit,
+ apr_size_t error_check_interval,
+ apr_pool_t *pool)
+{
+ return svn_ra_svn_create_conn5(sock, in_stream, out_stream,
+ compression_level, zero_copy_limit,
+ error_check_interval, 0, pool);
+}
+
+svn_ra_svn_conn_t *
svn_ra_svn_create_conn3(apr_socket_t *sock,
apr_file_t *in_file,
apr_file_t *out_file,
Modified: subversion/trunk/subversion/libsvn_ra_svn/editorp.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_svn/editorp.c?rev=1714330&r1=1714329&r2=1714330&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_svn/editorp.c (original)
+++ subversion/trunk/subversion/libsvn_ra_svn/editorp.c Sat Nov 14 14:09:18 2015
@@ -1106,6 +1106,10 @@ svn_error_t *svn_ra_svn_drive_editor2(sv
while (!state.done)
{
svn_pool_clear(subpool);
+
+ /* WRT to applying I/O limits, treat each editor command as a separate
+ * protocol command. */
+ svn_ra_svn__reset_command_io_counters(conn);
if (editor)
{
cmd_handler_t handler;
Modified: subversion/trunk/subversion/libsvn_ra_svn/marshal.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_svn/marshal.c?rev=1714330&r1=1714329&r2=1714330&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_svn/marshal.c (original)
+++ subversion/trunk/subversion/libsvn_ra_svn/marshal.c Sat Nov 14 14:09:18 2015
@@ -179,12 +179,13 @@ svn_ra_svn__to_private_array(const apr_a
/* --- CONNECTION INITIALIZATION --- */
-svn_ra_svn_conn_t *svn_ra_svn_create_conn4(apr_socket_t *sock,
+svn_ra_svn_conn_t *svn_ra_svn_create_conn5(apr_socket_t *sock,
svn_stream_t *in_stream,
svn_stream_t *out_stream,
int compression_level,
apr_size_t zero_copy_limit,
apr_size_t error_check_interval,
+ apr_uint64_t max_in,
apr_pool_t *result_pool)
{
svn_ra_svn_conn_t *conn;
@@ -204,6 +205,8 @@ svn_ra_svn_conn_t *svn_ra_svn_create_con
conn->written_since_error_check = 0;
conn->error_check_interval = error_check_interval;
conn->may_check_for_error = error_check_interval == 0;
+ conn->max_in = max_in;
+ conn->current_in = 0;
conn->block_handler = NULL;
conn->block_baton = NULL;
conn->capabilities = apr_hash_make(result_pool);
@@ -312,8 +315,27 @@ svn_error_t *svn_ra_svn__data_available(
return svn_ra_svn__stream_data_available(conn->stream, data_available);
}
+void
+svn_ra_svn__reset_command_io_counters(svn_ra_svn_conn_t *conn)
+{
+ conn->current_in = 0;
+}
+
+
/* --- WRITE BUFFER MANAGEMENT --- */
+/* Return an error object if CONN exceeded its send limits. */
+static svn_error_t *
+check_io_limits(svn_ra_svn_conn_t *conn)
+{
+ if (conn->max_in && (conn->current_in > conn->max_in))
+ return svn_error_create(SVN_ERR_RA_SVN_REQUEST_SIZE, NULL,
+ "The client request size exceeds the "
+ "configured limit");
+
+ return SVN_NO_ERROR;
+}
+
/* Write data to socket or output file as appropriate. */
static svn_error_t *writebuf_output(svn_ra_svn_conn_t *conn, apr_pool_t *pool,
const char *data, apr_size_t len)
@@ -441,12 +463,19 @@ static svn_error_t *readbuf_input(svn_ra
{
svn_ra_svn__session_baton_t *session = conn->session;
+ /* First, give the user a chance to cancel the request before we do. */
if (session && session->callbacks && session->callbacks->cancel_func)
SVN_ERR((session->callbacks->cancel_func)(session->callbacks_baton));
+ /* Limit our memory usage, if a limit has been configured. Note that
+ * we first read the whole request into memory before process it. */
+ SVN_ERR(check_io_limits(conn));
+
+ /* Actually fill the buffer. */
SVN_ERR(svn_ra_svn__stream_read(conn->stream, data, len));
if (*len == 0)
return svn_error_create(SVN_ERR_RA_SVN_CONNECTION_CLOSED, NULL, NULL);
+ conn->current_in += *len;
if (session)
{
@@ -1795,7 +1824,12 @@ svn_ra_svn__has_command(svn_boolean_t *h
svn_ra_svn_conn_t *conn,
apr_pool_t *pool)
{
- svn_error_t *err = svn_ra_svn__has_item(has_command, conn, pool);
+ svn_error_t *err;
+
+ /* Don't make whitespace between commands trigger I/O limitiations. */
+ svn_ra_svn__reset_command_io_counters(conn);
+
+ err = svn_ra_svn__has_item(has_command, conn, pool);
if (err && err->apr_err == SVN_ERR_RA_SVN_CONNECTION_CLOSED)
{
*terminated = TRUE;
@@ -1821,6 +1855,10 @@ svn_ra_svn__handle_command(svn_boolean_t
const svn_ra_svn__cmd_entry_t *command;
*terminate = FALSE;
+
+ /* Limit I/O for every command separately. */
+ svn_ra_svn__reset_command_io_counters(conn);
+
err = svn_ra_svn__read_tuple(conn, pool, "wl", &cmdname, ¶ms);
if (err)
{
@@ -1852,6 +1890,13 @@ svn_ra_svn__handle_command(svn_boolean_t
baton);
}
+ /* The command implementation may have swallowed or wrapped the I/O
+ * error not knowing that we may no longer be able to send data.
+ *
+ * So, check again for the limit violations and exit the command
+ * processing quickly if we may have truncated data. */
+ err = svn_error_compose_create(check_io_limits(conn), err);
+
*terminate = command->terminate;
}
else
Modified: subversion/trunk/subversion/libsvn_ra_svn/ra_svn.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_svn/ra_svn.h?rev=1714330&r1=1714329&r2=1714330&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_svn/ra_svn.h (original)
+++ subversion/trunk/subversion/libsvn_ra_svn/ra_svn.h Sat Nov 14 14:09:18 2015
@@ -96,6 +96,10 @@ struct svn_ra_svn_conn_st {
apr_size_t error_check_interval;
svn_boolean_t may_check_for_error;
+ /* I/O limits and tracking */
+ apr_uint64_t max_in;
+ apr_uint64_t current_in;
+
/* repository info */
const char *uuid;
const char *repos_root;
@@ -151,6 +155,12 @@ void svn_ra_svn__set_block_handler(svn_r
svn_error_t *svn_ra_svn__data_available(svn_ra_svn_conn_t *conn,
svn_boolean_t *data_available);
+/* Signal a new request / response pair on CONN. That resets the I/O
+ * counters we use to limit the size of individual requests / response pairs.
+ */
+void
+svn_ra_svn__reset_command_io_counters(svn_ra_svn_conn_t *conn);
+
/* CRAM-MD5 client implementation. */
svn_error_t *svn_ra_svn__cram_client(svn_ra_svn_conn_t *conn, apr_pool_t *pool,
const char *user, const char *password,
Modified: subversion/trunk/subversion/svnserve/serve.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svnserve/serve.c?rev=1714330&r1=1714329&r2=1714330&view=diff
==============================================================================
--- subversion/trunk/subversion/svnserve/serve.c (original)
+++ subversion/trunk/subversion/svnserve/serve.c Sat Nov 14 14:09:18 2015
@@ -4123,10 +4123,11 @@ serve_interruptable(svn_boolean_t *termi
/* create the connection, configure ports etc. */
connection->conn
- = svn_ra_svn_create_conn4(connection->usock, NULL, NULL,
+ = svn_ra_svn_create_conn5(connection->usock, NULL, NULL,
connection->params->compression_level,
connection->params->zero_copy_limit,
connection->params->error_check_interval,
+ connection->params->max_request_size,
connection->pool);
/* Construct server baton and open the repository for the first time. */
Modified: subversion/trunk/subversion/svnserve/server.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svnserve/server.h?rev=1714330&r1=1714329&r2=1714330&view=diff
==============================================================================
--- subversion/trunk/subversion/svnserve/server.h (original)
+++ subversion/trunk/subversion/svnserve/server.h Sat Nov 14 14:09:18 2015
@@ -152,6 +152,9 @@ typedef struct serve_params_t {
coming in from the client. */
apr_size_t error_check_interval;
+ /* If not 0, error out on requests exceeding this value. */
+ apr_uint64_t max_request_size;
+
/* Use virtual-host-based path to repo. */
svn_boolean_t vhost;
} serve_params_t;
Modified: subversion/trunk/subversion/svnserve/svnserve.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svnserve/svnserve.c?rev=1714330&r1=1714329&r2=1714330&view=diff
==============================================================================
--- subversion/trunk/subversion/svnserve/svnserve.c (original)
+++ subversion/trunk/subversion/svnserve/svnserve.c Sat Nov 14 14:09:18 2015
@@ -155,6 +155,15 @@ enum run_mode {
*/
#define ACCEPT_BACKLOG 128
+/* Default limit to the client request size in MBytes. This effectively
+ * limits the size of a paths and individual property values to about
+ * this value.
+ *
+ * Note that (MAX_REQUEST_SIZE + 4M) * THREADPOOL_MAX_SIZE is roughly
+ * the peak memory usage of the RA layer.
+ */
+#define MAX_REQUEST_SIZE 16
+
#ifdef WIN32
static apr_os_sock_t winservice_svnserve_accept_socket = INVALID_SOCKET;
@@ -210,6 +219,7 @@ void winservice_notify_stop(void)
#define SVNSERVE_OPT_MIN_THREADS 271
#define SVNSERVE_OPT_MAX_THREADS 272
#define SVNSERVE_OPT_BLOCK_READ 273
+#define SVNSERVE_OPT_MAX_REQUEST 274
/* Text macro because we can't use #ifdef sections inside a N_("...")
macro expansion. */
@@ -345,6 +355,19 @@ static const apr_getopt_option_t svnserv
"Default is " APR_STRINGIFY(THREADPOOL_MAX_SIZE) "."
ONLY_AVAILABLE_WITH_THEADS)},
#endif
+ {"max-request-size", SVNSERVE_OPT_MAX_REQUEST, 1,
+ N_("Maximum acceptable size of a client request in MB.\n"
+ " "
+ "This implicitly limits the length of paths and\n"
+ " "
+ "property values that can be sent to the server.\n"
+ " "
+ "Also the peak memory usage for protocol handling\n"
+ " "
+ "per server thread or sub-process.\n"
+ " "
+ "0 disables the size check; default is "
+ APR_STRINGIFY(MAX_REQUEST_SIZE) ".")},
{"foreground", SVNSERVE_OPT_FOREGROUND, 0,
N_("run in foreground (useful for debugging)\n"
" "
@@ -728,6 +751,7 @@ sub_main(int *exit_code, int argc, const
params.memory_cache_size = (apr_uint64_t)-1;
params.zero_copy_limit = 0;
params.error_check_interval = 4096;
+ params.max_request_size = MAX_REQUEST_SIZE * 0x100000;
while (1)
{
@@ -891,6 +915,10 @@ sub_main(int *exit_code, int argc, const
}
break;
+ case SVNSERVE_OPT_MAX_REQUEST:
+ params.max_request_size = 0x100000 * apr_strtoi64(arg, NULL, 0);
+ break;
+
case SVNSERVE_OPT_MIN_THREADS:
min_thread_count = (apr_size_t)apr_strtoi64(arg, NULL, 0);
break;
@@ -1039,10 +1067,11 @@ sub_main(int *exit_code, int argc, const
* the pool cleanup handlers that call sasl_dispose() (connection_pool)
* and sasl_done() (pool) are run in the right order. See issue #3664. */
connection_pool = svn_pool_create(pool);
- conn = svn_ra_svn_create_conn4(NULL, stdin_stream, stdout_stream,
+ conn = svn_ra_svn_create_conn5(NULL, stdin_stream, stdout_stream,
params.compression_level,
params.zero_copy_limit,
params.error_check_interval,
+ params.max_request_size,
connection_pool);
err = serve(conn, ¶ms, connection_pool);
svn_pool_destroy(connection_pool);