On Tue, 2012-01-24 at 18:10 +0100, Gergely Nagy wrote:
When using the syslog protocol, in case when we received an EAGAIN during an attempt to send the framing, the frame was not resent, resulting in protocol errors and invalid frame header messages on the other end.
This patch corrects the problem by not using log_proto_text_client_post to send framed messages, but a wrapper that can resume sending the frame too.
Signed-off-by: Fried Zoltan <deirf@balabit.hu> Signed-off-by: Gergely Nagy <algernon@balabit.hu> --- lib/logproto.c | 64 ++++++++++++++++++++++++++++++++++++------------------- 1 files changed, 42 insertions(+), 22 deletions(-)
diff --git a/lib/logproto.c b/lib/logproto.c index 16ca0d7..e2cf944 100644 --- a/lib/logproto.c +++ b/lib/logproto.c @@ -35,6 +35,26 @@ #include <sys/uio.h> #include <limits.h>
+ +typedef struct _LogProtoTextClient +{ + LogProto super; + guchar *partial; + gsize partial_len, partial_pos; +} LogProtoTextClient; + +#define LPFCS_FRAME_INIT 0 +#define LPFCS_FRAME_SEND 1 +#define LPFCS_MESSAGE_SEND 2 + +typedef struct _LogProtoFramedClient +{ + LogProtoTextClient super; + gint state; + gchar frame_hdr_buf[9]; + gint frame_hdr_len, frame_hdr_pos; +} LogProtoFramedClient; + gboolean log_proto_set_encoding(LogProto *self, const gchar *encoding) { @@ -70,14 +90,6 @@ log_proto_free(LogProto *s) g_free(s); }
- -typedef struct _LogProtoTextClient -{ - LogProto super; - guchar *partial; - gsize partial_len, partial_pos; -} LogProtoTextClient; - static gboolean log_proto_text_client_prepare(LogProto *s, gint *fd, GIOCondition *cond) { @@ -144,7 +156,7 @@ log_proto_text_client_flush(LogProto *s) * successfully sent this message, or if it should be resent by the caller. **/ static LogProtoStatus -log_proto_text_client_post(LogProto *s, guchar *msg, gsize msg_len, gboolean *consumed) +log_proto_client_post_writer(LogProto *s, guchar *msg, gsize msg_len, gboolean *consumed, gboolean syslog_proto) { LogProtoTextClient *self = (LogProtoTextClient *) s; gint rc; @@ -173,6 +185,14 @@ log_proto_text_client_post(LogProto *s, guchar *msg, gsize msg_len, gboolean *co */ return rc; } + else if (self->partial_len != 0 && !self->partial && syslog_proto) + { + LogProtoFramedClient *framed_self = (LogProtoFramedClient *) s; + self->partial_pos =0; + self->partial_len =0; + framed_self->state = LPFCS_FRAME_INIT; + return rc; + }
sorry, but I won't commit this. you are casting "self" to a subclass in a more generic class. I don't really see why this isn't handled by the "if" in log_proto_framed_client_post() right after the call to log_proto_text_client_post(). Here's the code that should do the same: case LPFCS_MESSAGE_SEND: rc = log_proto_text_client_post(s, msg, msg_len, consumed); /* NOTE: we don't check *consumed here, as we might have a pending * message in self->partial before we begin, in which case *consumed * will be FALSE. */ if (rc == LPS_SUCCESS && self->super.partial == NULL) { self->state = LPFCS_FRAME_INIT; } return rc; If a partial message is set and we get an EAGAIN, then log_proto_text_client_flush() returns LPS_SUCCESS and self->partial != NULL, which in turn leaves the state LPFCS_MESSAGE_SEND, causing the partial message to be resent. What am I missing? can you describe the concrete scenario that triggers the bug? Thanks. -- Bazsi