Author: kotkov
Date: Mon Dec 22 19:30:13 2025
New Revision: 1930807
Log:
ra_serf: Implement and start using a default readline handler for
custom bucket types.
Before this change, we were making an assumption that the calling site
is not going to call bucket->readline() on our custom bucket types.
This is not something a bucket type has control over, and the assumption
may not hold in the future.
To solve this issue, we provide a default bucket readline implementation,
modeled over the current implementation of serf_default_readline() in
serf trunk.
* subversion/libsvn_ra_serf/ra_serf.h
(svn_ra_serf__default_readline): New function.
* subversion/libsvn_ra_serf/util.c
(svn_ra_serf__default_readline): Implement based on the current
implementation of serf_default_readline() in serf trunk.
* subversion/libsvn_ra_serf/eagain_bucket.c
* subversion/libsvn_ra_serf/sb_bucket.c:
Use svn_ra_serf__default_readline() as the readline handler.
Modified:
subversion/trunk/subversion/libsvn_ra_serf/eagain_bucket.c
subversion/trunk/subversion/libsvn_ra_serf/ra_serf.h
subversion/trunk/subversion/libsvn_ra_serf/sb_bucket.c
subversion/trunk/subversion/libsvn_ra_serf/util.c
Modified: subversion/trunk/subversion/libsvn_ra_serf/eagain_bucket.c
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_serf/eagain_bucket.c Mon Dec 22
18:45:03 2025 (r1930806)
+++ subversion/trunk/subversion/libsvn_ra_serf/eagain_bucket.c Mon Dec 22
19:30:13 2025 (r1930807)
@@ -66,21 +66,6 @@ eagain_bucket_read(serf_bucket_t *bucket
return APR_EAGAIN;
}
-#if !SERF_VERSION_AT_LEAST(1, 4, 0)
-static apr_status_t
-eagain_bucket_readline(serf_bucket_t *bucket,
- int acceptable,
- int *found,
- const char **data,
- apr_size_t *len)
-{
- /* ### for now, we know callers won't use this function. */
- svn_error_clear(svn_error__malfunction(TRUE, __FILE__, __LINE__,
- "Not implemented."));
- return APR_ENOTIMPL;
-}
-#endif
-
static apr_status_t
eagain_bucket_peek(serf_bucket_t *bucket,
@@ -99,11 +84,7 @@ eagain_bucket_peek(serf_bucket_t *bucket
static const serf_bucket_type_t delay_bucket_vtable = {
"BUF-EAGAIN",
eagain_bucket_read,
-#if SERF_VERSION_AT_LEAST(1, 4, 0)
- serf_default_readline,
-#else
- eagain_bucket_readline,
-#endif
+ svn_ra_serf__default_readline,
serf_default_read_iovec,
serf_default_read_for_sendfile,
serf_default_read_bucket,
Modified: subversion/trunk/subversion/libsvn_ra_serf/ra_serf.h
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_serf/ra_serf.h Mon Dec 22
18:45:03 2025 (r1930806)
+++ subversion/trunk/subversion/libsvn_ra_serf/ra_serf.h Mon Dec 22
19:30:13 2025 (r1930807)
@@ -1667,6 +1667,15 @@ svn_ra_serf__create_stream_bucket(svn_st
svn_ra_serf__stream_bucket_errfunc_t errfunc,
void *errfunc_baton);
+/* Default implementation of the bucket readline function, see
+ serf_bucket_type_t.readline. This function will use the bucket
+ read function, when possible optimized by the bucket peek function
+ to return the requested result. */
+apr_status_t
+svn_ra_serf__default_readline(serf_bucket_t *bucket, int acceptable,
+ int *found,
+ const char **data, apr_size_t *len);
+
#if defined(SVN_DEBUG)
/* Wrapper macros to collect file and line information */
#define svn_ra_serf__wrap_err \
Modified: subversion/trunk/subversion/libsvn_ra_serf/sb_bucket.c
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_serf/sb_bucket.c Mon Dec 22
18:45:03 2025 (r1930806)
+++ subversion/trunk/subversion/libsvn_ra_serf/sb_bucket.c Mon Dec 22
19:30:13 2025 (r1930807)
@@ -117,19 +117,6 @@ sb_bucket_read(serf_bucket_t *bucket, ap
return *data == NULL ? APR_EOF : APR_SUCCESS;
}
-#if !SERF_VERSION_AT_LEAST(1, 4, 0)
-static apr_status_t
-sb_bucket_readline(serf_bucket_t *bucket, int acceptable,
- int *found,
- const char **data, apr_size_t *len)
-{
- /* ### for now, we know callers won't use this function. */
- svn_error_clear(svn_error__malfunction(TRUE, __FILE__, __LINE__,
- "Not implemented."));
- return APR_ENOTIMPL;
-}
-#endif
-
static apr_status_t
sb_bucket_peek(serf_bucket_t *bucket,
@@ -160,11 +147,7 @@ sb_bucket_peek(serf_bucket_t *bucket,
static const serf_bucket_type_t sb_bucket_vtable = {
"SPILLBUF",
sb_bucket_read,
-#if SERF_VERSION_AT_LEAST(1, 4, 0)
- serf_default_readline,
-#else
- sb_bucket_readline,
-#endif
+ svn_ra_serf__default_readline,
serf_default_read_iovec,
serf_default_read_for_sendfile,
serf_default_read_bucket,
Modified: subversion/trunk/subversion/libsvn_ra_serf/util.c
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_serf/util.c Mon Dec 22 18:45:03
2025 (r1930806)
+++ subversion/trunk/subversion/libsvn_ra_serf/util.c Mon Dec 22 19:30:13
2025 (r1930807)
@@ -39,6 +39,7 @@
#include "svn_string.h"
#include "svn_props.h"
#include "svn_dirent_uri.h"
+#include "svn_sorts.h"
#include "../libsvn_ra/ra_loader.h"
#include "private/svn_dep_compat.h"
@@ -2154,3 +2155,128 @@ svn_ra_serf__get_dirent_props(apr_uint32
return props;
}
+static apr_status_t
+bucket_limited_readline(serf_bucket_t *bucket, int acceptable,
+ apr_size_t requested, int *found,
+ const char **data, apr_size_t *len)
+{
+ apr_status_t status;
+ const char *peek_data;
+ apr_size_t peek_len;
+
+ status = bucket->type->peek(bucket, &peek_data, &peek_len);
+ if (SERF_BUCKET_READ_ERROR(status))
+ return status;
+
+ if (peek_len == 0)
+ {
+ /* peek() returned no data. */
+
+ /* ... if that's because the bucket has no data, then we're done. */
+ if (APR_STATUS_IS_EOF(status))
+ {
+ *found = SERF_NEWLINE_NONE;
+ *len = 0;
+ return APR_EOF;
+ }
+
+ /* We can only read and return a single character.
+
+ For example, if we tried reading 2 characters seeking CRLF, and
+ got CR followed by 'a', then we have over-read the line, and
+ consumed a character from the next line. Bad.
+
+ The only exception is when we *only* allow CRLF as newline. In that
+ case CR followed by 'a' would just be raw line data, not a line
+ break followed by data. If we allow any other type of newline we
+ can't use this trick.
+ */
+
+ if ((acceptable & SERF_NEWLINE_ANY) == SERF_NEWLINE_CRLF)
+ requested = MIN(requested, 2); /* Only CRLF is allowed */
+ else
+ requested = MIN(requested, 1);
+ }
+ else
+ {
+ /* peek_len > 0 */
+
+ const char *cr = NULL;
+ const char *lf = NULL;
+
+ if (peek_len > requested)
+ peek_len = requested;
+
+ if ((acceptable & SERF_NEWLINE_CR) || (acceptable & SERF_NEWLINE_CRLF))
+ cr = memchr(peek_data, '\r', peek_len);
+ if ((acceptable & SERF_NEWLINE_LF))
+ lf = memchr(peek_data, '\n', peek_len);
+
+ if (cr && lf)
+ cr = MIN(cr, lf);
+ else if (lf)
+ cr = lf;
+
+ /* ### When we are only looking for CRLF we may return too small
+ chunks here when the data contains CR or LF without the other.
+ That isn't incorrect, but it could be optimized.
+
+ ### But as that case is not common, the caller has to assume
+ partial reads anyway and this is just a not very inefficient
+ fallback implementation...
+
+ Let's make the buffering in the caller handle that case
+ for now. */
+
+ if (cr && *cr == '\r' && (acceptable & SERF_NEWLINE_CRLF) &&
+ ((cr + 1) < (peek_data + peek_len)) && *(cr + 1) == '\n')
+ {
+ requested = (cr + 2) - peek_data;
+ }
+ else if (cr)
+ requested = (cr + 1) - peek_data;
+ else
+ requested = peek_len;
+ }
+
+ status = bucket->type->read(bucket, requested, data, len);
+ if (SERF_BUCKET_READ_ERROR(status))
+ return status;
+
+ if (*len == 0)
+ {
+ *found = SERF_NEWLINE_NONE;
+ }
+ else if ((acceptable & SERF_NEWLINE_CRLF) && *len >= 2 &&
+ (*data)[*len - 1] == '\n' && (*data)[*len - 2] == '\r')
+ {
+ *found = SERF_NEWLINE_CRLF;
+ }
+ else if ((acceptable & SERF_NEWLINE_LF) && (*data)[*len - 1] == '\n')
+ {
+ *found = SERF_NEWLINE_LF;
+ }
+ else if ((acceptable & (SERF_NEWLINE_CRLF | SERF_NEWLINE_CR)) &&
+ (*data)[*len - 1] == '\r')
+ {
+ *found = (acceptable & (SERF_NEWLINE_CRLF)) ? SERF_NEWLINE_CRLF_SPLIT
+ : SERF_NEWLINE_CR;
+ }
+ else
+ *found = SERF_NEWLINE_NONE;
+
+ return status;
+}
+
+apr_status_t
+svn_ra_serf__default_readline(serf_bucket_t *bucket, int acceptable,
+ int *found,
+ const char **data, apr_size_t *len)
+{
+#if SERF_VERSION_AT_LEAST(1, 4, 0)
+ return serf_default_readline(bucket, acceptable, found, data, len);
+#else
+ return bucket_limited_readline(bucket, acceptable, SERF_READ_ALL_AVAIL,
+ found, data, len);
+#endif
+}
