From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id C993026516 for ; Thu, 29 Mar 2018 03:40:27 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id WUGkeLaH7yzN for ; Thu, 29 Mar 2018 03:40:27 -0400 (EDT) Received: from smtp33.i.mail.ru (smtp33.i.mail.ru [94.100.177.93]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 7EA2921D8D for ; Thu, 29 Mar 2018 03:40:27 -0400 (EDT) From: Georgy Kirichenko Subject: [tarantool-patches] Re: [log 1/1] log: Fix logging large objects Date: Thu, 29 Mar 2018 10:40:18 +0300 Message-ID: <2045757.3eQz9zbCrq@home.lan> In-Reply-To: <5644fb6343e31f6e136071bb5659f893ac176400.1522233290.git.imarkov@tarantool.org> References: <5644fb6343e31f6e136071bb5659f893ac176400.1522233290.git.imarkov@tarantool.org> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="nextPart1638507.XM4I1PsMfM"; micalg="pgp-sha256"; protocol="application/pgp-signature" Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-subscribe: List-owner: List-post: List-archive: To: tarantool-patches@freelists.org Cc: Ilya Markov --nextPart1638507.XM4I1PsMfM Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" The patch itself looks Ok, but there is two tests failed on travis for the branch, please check it. Also a branch url must be included just bellow `---' On Wednesday, March 28, 2018 1:35:14 PM MSK Ilya Markov wrote: > The bug was that logging we passed to function write > number of bytes which may be more than size of buffer. > This may happen because formatting log string we use vsnprintf which > returns number of bytes would be written to buffer, not the actual > number. > > Fix this with limiting number of bytes passing to write function. > > Close #3248 > --- > src/say.c | 24 ++++++++++++++++++------ > test/app-tap/logger.test.lua | 9 +++++++-- > 2 files changed, 25 insertions(+), 8 deletions(-) > > diff --git a/src/say.c b/src/say.c > index b92514d..8a8d46a 100644 > --- a/src/say.c > +++ b/src/say.c > @@ -842,6 +842,18 @@ say_format_syslog(struct log *log, char *buf, int len, > int level, const char *fi enum { SAY_BUF_LEN_MAX = 16 * 1024 }; > static __thread char buf[SAY_BUF_LEN_MAX]; > > +/** > + * Wrapper over write which ensures, that writes not more than buffer size. > + */ > +static ssize_t > +safe_write(int fd, const char *buf, int size) > +{ > + /* Writes at most SAY_BUF_LEN_MAX - 1 > + * (1 byte was taken for 0 byte in vsnprintf). > + */ > + return write(fd, buf, MIN(size, SAY_BUF_LEN_MAX - 1)); > +} > + > static void > say_default(int level, const char *filename, int line, const char *error, > const char *format, ...) > @@ -852,7 +864,7 @@ say_default(int level, const char *filename, int line, > const char *error, int total = log_vsay(log_default, level, filename, > line, error, format, ap); > if (level == S_FATAL && log_default->fd != STDERR_FILENO) { > - ssize_t r = write(STDERR_FILENO, buf, total); > + ssize_t r = safe_write(STDERR_FILENO, buf, total); > (void) r; /* silence gcc warning */ > } > > @@ -870,7 +882,7 @@ write_to_file(struct log *log, int total) > log->type == SAY_LOGGER_PIPE || > log->type == SAY_LOGGER_STDERR); > assert(total >= 0); > - ssize_t r = write(log->fd, buf, total); > + ssize_t r = safe_write(log->fd, buf, total); > (void) r; /* silence gcc warning */ > } > > @@ -882,7 +894,7 @@ write_to_syslog(struct log *log, int total) > { > assert(log->type == SAY_LOGGER_SYSLOG); > assert(total >= 0); > - if (log->fd < 0 || write(log->fd, buf, total) <= 0) { > + if (log->fd < 0 || safe_write(log->fd, buf, total) <= 0) { > /* > * Try to reconnect, if write to syslog has > * failed. Syslog write can fail, if, for example, > @@ -899,7 +911,7 @@ write_to_syslog(struct log *log, int total) > * it would block thread. Try to reconnect > * on next vsay(). > */ > - ssize_t r = write(log->fd, buf, total); > + ssize_t r = safe_write(log->fd, buf, total); > (void) r; /* silence gcc warning */ > } > } > @@ -1041,11 +1053,11 @@ log_vsay(struct log *log, int level, const char > *filename, int line, case SAY_LOGGER_SYSLOG: > write_to_syslog(log, total); > if (level == S_FATAL && log->fd != STDERR_FILENO) > - (void) write(STDERR_FILENO, buf, total); > + (void) safe_write(STDERR_FILENO, buf, total); > break; > case SAY_LOGGER_BOOT: > { > - ssize_t r = write(STDERR_FILENO, buf, total); > + ssize_t r = safe_write(STDERR_FILENO, buf, total); > (void) r; /* silence gcc warning */ > break; > } > diff --git a/test/app-tap/logger.test.lua b/test/app-tap/logger.test.lua > index 72d13d3..85f6166 100755 > --- a/test/app-tap/logger.test.lua > +++ b/test/app-tap/logger.test.lua > @@ -1,7 +1,7 @@ > #!/usr/bin/env tarantool > > local test = require('tap').test('log') > -test:plan(23) > +test:plan(24) > > -- > -- Check that Tarantool creates ADMIN session for #! script > @@ -91,6 +91,11 @@ local line = file:read() > message = json.decode(line) > test:is(message.message, "this is \"", "check message with escaped > character") > > +-- gh-3248 trash in log file with logging large objects > +log.info(string.rep('a', 32000)) > +line = file:read() > +test:ok(line:len() < 20000, "big line truncated") > + > log.info("json") > local line = file:read() > message = json.decode(line) > @@ -100,7 +105,7 @@ log.info("hello") > line = file:read() > test:ok(not line:match("{"), "log change format") > s, e = pcall(log.log_format, "non_format") > -test:ok(not s) > +test:ok(not s, "bad format") > file:close() > > log.log_format("json") --nextPart1638507.XM4I1PsMfM Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part. Content-Transfer-Encoding: 7Bit -----BEGIN PGP SIGNATURE----- iQEzBAABCAAdFiEEBJFDbU76LsBbgHBsvKOmCX79zb4FAlq8mGIACgkQvKOmCX79 zb670AgAj1kpnEnmHfaPgIiEh9VHmZ+IqC2PiyDSFbrSFOyMBRA80Wosbc+NdSLF QFXxZJhzDp/QptATfZItGhp+xovVCG4UVS+cRu2EakNqJc/Pgv0ki+tAMmOziM2t 4XzM0IZCcnKU6ODQ0IfVUwf7fj59nUCG+YJ2mD0mAW8mHx7eZ/yYgQFrog6yXaYp ME9VHoBJ4eI5Ffw0Hst4ewAUOYg7y4YnGCNGbsFDWtmuJ1K99KoTxMWE8VXd1wdn 7MByAFKraC/bxhmrMh/jEx5sreqvdTJ57nzVl2zvzpIxdBaoSE7uzhzj/egCiMSl N6fEe79uvKlXcsMHbUKL6W1wm15jyA== =Rrd5 -----END PGP SIGNATURE----- --nextPart1638507.XM4I1PsMfM--