[evolution-data-server/eds-nonblocking] IMAPX: Buffer a full response line before parsing.
- From: Matthew Barnes <mbarnes src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [evolution-data-server/eds-nonblocking] IMAPX: Buffer a full response line before parsing.
- Date: Thu, 27 Mar 2014 16:29:32 +0000 (UTC)
commit e8015389e09cfa33606da55d1e4a23f973d777d1
Author: Matthew Barnes <mbarnes redhat com>
Date: Sat Mar 22 19:31:40 2014 -0400
IMAPX: Buffer a full response line before parsing.
CamelIMAPXInputStream (and CamelIMAPXStream in older releases) was
trying to immediately parse whatever data it got from a read() call.
The problem is sometimes an IMAP response line can be split across
multiple Ethernet packets.
This confuses the parser into thinking it's reached the end of input,
and triggers an error and subsequent disconnect from the IMAP server.
We had a case of this recently from a user with a large number of
mailboxes -- enough that one of the LIST responses got split across
multiple Ethernet packets.
This commit attempts to passively buffer incoming data from the IMAP
socket until a full response line is detected, and only then is the
buffered response line parsed.
Note that response line detection is pretty primitive, and literals
containing CRLF sequences can trick it. Therefore I'm leaving the
imapx_input_stream_fill() logic in place during tokenization until
we can replace our ad-hoc lexer with something more sophisticated
like Flex.
camel/providers/imapx/camel-imapx-input-stream.c | 214 ++++++++++++++++++----
camel/providers/imapx/camel-imapx-input-stream.h | 10 +
camel/providers/imapx/camel-imapx-server.c | 62 ++++++-
3 files changed, 245 insertions(+), 41 deletions(-)
---
diff --git a/camel/providers/imapx/camel-imapx-input-stream.c
b/camel/providers/imapx/camel-imapx-input-stream.c
index ebc8af9..a88ab45 100644
--- a/camel/providers/imapx/camel-imapx-input-stream.c
+++ b/camel/providers/imapx/camel-imapx-input-stream.c
@@ -58,6 +58,61 @@ G_DEFINE_TYPE_WITH_CODE (
G_TYPE_POLLABLE_INPUT_STREAM,
camel_imapx_input_stream_pollable_init))
+static void
+imapx_input_stream_grow (CamelIMAPXInputStream *is,
+ guint len,
+ guchar **bufptr,
+ guchar **tokptr)
+{
+ guchar *oldtok = is->priv->tokenbuf;
+ guchar *oldbuf = is->priv->buf;
+
+ do {
+ is->priv->bufsize <<= 1;
+ } while (is->priv->bufsize <= len);
+
+ is->priv->tokenbuf = g_realloc (
+ is->priv->tokenbuf,
+ is->priv->bufsize + 1);
+ if (tokptr)
+ *tokptr = is->priv->tokenbuf + (*tokptr - oldtok);
+ if (is->priv->unget)
+ is->priv->unget_token =
+ is->priv->tokenbuf +
+ (is->priv->unget_token - oldtok);
+
+ is->priv->buf = g_realloc (is->priv->buf, is->priv->bufsize + 1);
+ is->priv->ptr = is->priv->buf + (is->priv->ptr - oldbuf);
+ is->priv->end = is->priv->buf + (is->priv->end - oldbuf);
+ if (bufptr)
+ *bufptr = is->priv->buf + (*bufptr - oldbuf);
+}
+
+static gsize
+imapx_input_stream_prep_for_read (CamelIMAPXInputStream *is)
+{
+ gsize bytes_buffered;
+ gsize bytes_available;
+
+ bytes_buffered = is->priv->end - is->priv->ptr;
+ bytes_available = is->priv->bufsize - bytes_buffered;
+
+ /* If no bytes are available, expand the buffer. */
+ if (bytes_available == 0) {
+ imapx_input_stream_grow (is, is->priv->bufsize, NULL, NULL);
+ bytes_available = is->priv->bufsize - bytes_buffered;
+ }
+
+ /* Shift any buffered data to the front of the buffer. */
+ if (is->priv->ptr != is->priv->buf) {
+ memcpy (is->priv->buf, is->priv->ptr, bytes_buffered);
+ is->priv->end = is->priv->buf + bytes_buffered;
+ is->priv->ptr = is->priv->buf;
+ }
+
+ return bytes_available;
+}
+
static gint
imapx_input_stream_fill (CamelIMAPXInputStream *is,
GCancellable *cancellable,
@@ -245,36 +300,6 @@ camel_imapx_input_stream_init (CamelIMAPXInputStream *is)
is->priv->tokenbuf = g_malloc (is->priv->bufsize + 1);
}
-static void
-camel_imapx_input_stream_grow (CamelIMAPXInputStream *is,
- guint len,
- guchar **bufptr,
- guchar **tokptr)
-{
- guchar *oldtok = is->priv->tokenbuf;
- guchar *oldbuf = is->priv->buf;
-
- do {
- is->priv->bufsize <<= 1;
- } while (is->priv->bufsize <= len);
-
- is->priv->tokenbuf = g_realloc (
- is->priv->tokenbuf,
- is->priv->bufsize + 1);
- if (tokptr)
- *tokptr = is->priv->tokenbuf + (*tokptr - oldtok);
- if (is->priv->unget)
- is->priv->unget_token =
- is->priv->tokenbuf +
- (is->priv->unget_token - oldtok);
-
- is->priv->buf = g_realloc (is->priv->buf, is->priv->bufsize + 1);
- is->priv->ptr = is->priv->buf + (is->priv->ptr - oldbuf);
- is->priv->end = is->priv->buf + (is->priv->end - oldbuf);
- if (bufptr)
- *bufptr = is->priv->buf + (*bufptr - oldbuf);
-}
-
G_DEFINE_QUARK (camel-imapx-error-quark, camel_imapx_error)
/**
@@ -309,6 +334,127 @@ camel_imapx_input_stream_buffered (CamelIMAPXInputStream *is)
return is->priv->end - is->priv->ptr;
}
+/**
+ * camel_imapx_input_stream_has_response:
+ * @is: a #CamelIMAPXInputStream
+ *
+ * Returns whether the stream has a full IMAP response line buffered.
+ *
+ * Returns: whether a full response line is buffered
+ *
+ * Since: 3.14
+ **/
+gboolean
+camel_imapx_input_stream_has_response (CamelIMAPXInputStream *is)
+{
+ gboolean prev_was_cr = FALSE;
+ guchar *cp;
+
+ g_return_val_if_fail (CAMEL_IS_IMAPX_INPUT_STREAM (is), FALSE);
+
+ /* Look for a buffered CRLF sequence. */
+
+ for (cp = is->priv->ptr; cp < is->priv->end; cp++) {
+ if (*cp == 10 && prev_was_cr)
+ return TRUE;
+ prev_was_cr = (*cp == 13);
+ }
+
+ return FALSE;
+}
+
+/**
+ * camel_imapx_input_stream_wait_for_response:
+ * @is: a #CamelIMAPXInputStream
+ * @cancellable: optional #GCancellable object, or %NULL
+ * @error: return location for a #GError, or %NULL
+ *
+ * Blocks until a full IMAP response line has been read from the input stream
+ * and buffered internally, or until a cancellation or I/O error occurs.
+ *
+ * Returns: %TRUE if a full response line is buffered, %FALSE if an error
+ * occurred
+ *
+ * Since: 3.14
+ **/
+gboolean
+camel_imapx_input_stream_wait_for_response (CamelIMAPXInputStream *is,
+ GCancellable *cancellable,
+ GError **error)
+{
+ GInputStream *base_stream;
+
+ g_return_val_if_fail (CAMEL_IS_IMAPX_INPUT_STREAM (is), FALSE);
+
+ base_stream = g_filter_input_stream_get_base_stream (
+ G_FILTER_INPUT_STREAM (is));
+
+ while (!camel_imapx_input_stream_has_response (is)) {
+ gsize bytes_available;
+ gssize bytes_read;
+
+ bytes_available = imapx_input_stream_prep_for_read (is);
+
+ bytes_read = g_input_stream_read (
+ base_stream,
+ is->priv->end, bytes_available,
+ cancellable, error);
+
+ if (bytes_read > 0) {
+ is->priv->end += bytes_read;
+ } else {
+ if (bytes_read == 0) {
+ /* XXX Not sure if G_IO_ERROR_CLOSED is the
+ * best error code but it's better than
+ * CAMEL_ERROR_GENERIC. */
+ g_set_error_literal (
+ error, G_IO_ERROR, G_IO_ERROR_CLOSED,
+ _("Source stream returned no data"));
+ }
+
+ return FALSE;
+ }
+ }
+
+ return TRUE;
+}
+
+/**
+ * camel_imapx_input_stream_read_nonblocking:
+ * @is: a #CamelIMAPXInputStream
+ * @cancellable: optional #GCancellable object, or %NULL
+ * @error: return location for a #GError, or %NULL
+ *
+ * Similar to g_pollable_input_stream_read_nonblocking(), except the data
+ * read from the stream is buffered internally.
+ *
+ * Returns: the number of bytes read, or -1 on error
+ *
+ * Since: 3.14
+ **/
+gssize
+camel_imapx_input_stream_read_nonblocking (CamelIMAPXInputStream *is,
+ GCancellable *cancellable,
+ GError **error)
+{
+ gsize bytes_available;
+ gssize bytes_read;
+
+ g_return_val_if_fail (CAMEL_IS_IMAPX_INPUT_STREAM (is), -1);
+
+ bytes_available = imapx_input_stream_prep_for_read (is);
+
+ bytes_read = g_pollable_input_stream_read_nonblocking (
+ G_POLLABLE_INPUT_STREAM (is),
+ is->priv->end, bytes_available,
+ cancellable, error);
+
+ if (bytes_read > 0)
+ is->priv->end += bytes_read;
+
+ return bytes_read;
+}
+
/* FIXME: these should probably handle it themselves,
* and get rid of the token interface? */
gboolean
@@ -377,7 +523,7 @@ camel_imapx_input_stream_astring (CamelIMAPXInputStream *is,
case IMAPX_TOK_LITERAL:
if (len >= is->priv->bufsize)
- camel_imapx_input_stream_grow (is, len, NULL, NULL);
+ imapx_input_stream_grow (is, len, NULL, NULL);
p = is->priv->tokenbuf;
camel_imapx_input_stream_set_literal (is, len);
do {
@@ -426,7 +572,7 @@ camel_imapx_input_stream_nstring (CamelIMAPXInputStream *is,
case IMAPX_TOK_LITERAL:
if (len >= is->priv->bufsize)
- camel_imapx_input_stream_grow (is, len, NULL, NULL);
+ imapx_input_stream_grow (is, len, NULL, NULL);
p = is->priv->tokenbuf;
camel_imapx_input_stream_set_literal (is, len);
do {
@@ -718,7 +864,7 @@ camel_imapx_input_stream_token (CamelIMAPXInputStream *is,
if (c == '\n' || c == '\r')
goto protocol_error;
if (o >= oe) {
- camel_imapx_input_stream_grow (is, 0, &p, &o);
+ imapx_input_stream_grow (is, 0, &p, &o);
oe = is->priv->tokenbuf + is->priv->bufsize - 1;
e = is->priv->end;
}
@@ -751,7 +897,7 @@ camel_imapx_input_stream_token (CamelIMAPXInputStream *is,
}
if (o >= oe) {
- camel_imapx_input_stream_grow (is, 0, &p, &o);
+ imapx_input_stream_grow (is, 0, &p, &o);
oe = is->priv->tokenbuf + is->priv->bufsize - 1;
e = is->priv->end;
}
diff --git a/camel/providers/imapx/camel-imapx-input-stream.h
b/camel/providers/imapx/camel-imapx-input-stream.h
index 4c953bb..696ed27 100644
--- a/camel/providers/imapx/camel-imapx-input-stream.h
+++ b/camel/providers/imapx/camel-imapx-input-stream.h
@@ -71,6 +71,16 @@ GType camel_imapx_input_stream_get_type
GInputStream * camel_imapx_input_stream_new (GInputStream *base_stream);
gint camel_imapx_input_stream_buffered
(CamelIMAPXInputStream *is);
+gboolean camel_imapx_input_stream_has_response
+ (CamelIMAPXInputStream *is);
+gboolean camel_imapx_input_stream_wait_for_response
+ (CamelIMAPXInputStream *is,
+ GCancellable *cancellable,
+ GError **error);
+gssize camel_imapx_input_stream_read_nonblocking
+ (CamelIMAPXInputStream *is,
+ GCancellable *cancellable,
+ GError **error);
camel_imapx_token_t
camel_imapx_input_stream_token (CamelIMAPXInputStream *is,
diff --git a/camel/providers/imapx/camel-imapx-server.c b/camel/providers/imapx/camel-imapx-server.c
index a636dd0..cd56334 100644
--- a/camel/providers/imapx/camel-imapx-server.c
+++ b/camel/providers/imapx/camel-imapx-server.c
@@ -3542,8 +3542,14 @@ imapx_command_run (CamelIMAPXServer *is,
imapx_command_start (is, ic);
QUEUE_UNLOCK (is);
- while (success && ic->status == NULL)
- success = imapx_step (is, input_stream, cancellable, error);
+ while (success && ic->status == NULL) {
+ success = camel_imapx_input_stream_wait_for_response (
+ CAMEL_IMAPX_INPUT_STREAM (input_stream),
+ cancellable, error);
+ if (success)
+ success = imapx_step (
+ is, input_stream, cancellable, error);
+ }
if (is->literal == ic)
is->literal = NULL;
@@ -4597,6 +4603,15 @@ connected:
input_stream = camel_imapx_server_ref_input_stream (is);
+ success = camel_imapx_input_stream_wait_for_response (
+ CAMEL_IMAPX_INPUT_STREAM (input_stream),
+ cancellable, error);
+
+ if (!success) {
+ g_object_unref (input_stream);
+ goto exit;
+ }
+
tok = camel_imapx_input_stream_token (
CAMEL_IMAPX_INPUT_STREAM (input_stream),
&token, &len, cancellable, error);
@@ -7599,6 +7614,7 @@ imapx_ready_to_read (GInputStream *input_stream,
{
GOutputStream *output_stream;
GCancellable *cancellable;
+ gssize bytes_read;
GError *local_error = NULL;
/* XXX Don't use the passed in GInputStream because that's
@@ -7610,13 +7626,45 @@ imapx_ready_to_read (GInputStream *input_stream,
cancellable = g_weak_ref_get (&is->priv->parser_cancellable);
- while (imapx_step (is, input_stream, cancellable, &local_error)) {
- gint bytes_buffered;
+ bytes_read = camel_imapx_input_stream_read_nonblocking (
+ CAMEL_IMAPX_INPUT_STREAM (input_stream),
+ cancellable, &local_error);
- bytes_buffered = camel_imapx_input_stream_buffered (
+ if (local_error != NULL) {
+ gboolean ignore_error;
+
+ /* XXX Not sure why this callback is invoked if there's no
+ * data ready to read, but it happens. Check for that
+ * condition and clear the error so we return silently. */
+ ignore_error = g_error_matches (
+ local_error, G_IO_ERROR, G_IO_ERROR_WOULD_BLOCK);
+
+ if (ignore_error)
+ g_clear_error (&local_error);
+
+ } else if (bytes_read == 0) {
+ /* XXX Not sure if G_IO_ERROR_CLOSED is the
+ * best error code but it's better than
+ * CAMEL_ERROR_GENERIC. */
+ local_error = g_error_new_literal (
+ G_IO_ERROR, G_IO_ERROR_CLOSED,
+ _("Source stream returned no data"));
+
+ } else {
+ gboolean success = TRUE;
+ gboolean has_response;
+
+ /* Only step if we have a full response line. */
+
+ has_response = camel_imapx_input_stream_has_response (
CAMEL_IMAPX_INPUT_STREAM (input_stream));
- if (bytes_buffered == 0)
- break;
+
+ while (success && has_response) {
+ success = imapx_step (
+ is, input_stream, cancellable, &local_error);
+ has_response = camel_imapx_input_stream_has_response (
+ CAMEL_IMAPX_INPUT_STREAM (input_stream));
+ }
}
if (g_cancellable_is_cancelled (cancellable)) {
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
