[PATCH] Logging of files with broken encoding

Ralph Böhme slow at samba.org
Tue Jun 9 05:41:42 MDT 2015


Hi,

attached is a patchset that moves a level 0 catch-all debug statement
from behind the switch clause inside the switch clause as default
case. This fixes an issue that resulted in the log being flooded with
those messages in case someone puts a file with an illegal UTF8
encoding (eg '\xA0test') in a toplevel directory:

[2015/05/12 18:17:26.865144,  0] ../lib/util/charset/convert_string.c:438(convert_string_talloc_handle)
  Conversion error: Incomplete multibyte sequence(<A0>test)

There's already an explicit EILSEQ case with a log message at loglevel
3, so afaict the level 0 log statement after the switch clause doesn't
make sense.

Otoh, I'd think it would be worthwhile logging a useful diagnostic
with the full file patch which is what patch 2/2 does.

As a result, with both patches in place we'd write this to the logs:

[2015/06/09 13:18:51.429654,  1, pid=3461] ../source3/smbd/trans2.c:2379(smbd_dirptr_lanman2_entry)
  Conversion error: illegal character: dir/test?

Thoughts?

Thanks!
-Ralph

-- 
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-370000-0, fax: +49-551-370000-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.sernet.de,mailto:kontakt@sernet.de
-------------- next part --------------
From dadce7f061365ce0a5c63f91ba1885ed3c89ee8b Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Wed, 3 Jun 2015 16:58:22 +0200
Subject: [PATCH 1/2] lib/util/charset: fix conversion failure logging

Move catch-all debug statement with loglevel 0 from behind the switch
clause into the switch clause as default case. Fixes an issue that
resulted in the log being flooded with level 0 messages in case someone
put a file with an illegal UTF8 encoding (eg '\xA0test') on the server.

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 lib/util/charset/convert_string.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lib/util/charset/convert_string.c b/lib/util/charset/convert_string.c
index 2e66680..50065f8 100644
--- a/lib/util/charset/convert_string.c
+++ b/lib/util/charset/convert_string.c
@@ -434,8 +434,10 @@ bool convert_string_talloc_handle(TALLOC_CTX *ctx, struct smb_iconv_handle *ic,
 				reason="Illegal multibyte sequence";
 				DEBUG(3,("convert_string_talloc: Conversion error: %s(%s)\n",reason,inbuf));
 				break;
+			default:
+				DEBUG(0,("Conversion error: %s(%s)\n",reason,inbuf));
+				break;
 		}
-		DEBUG(0,("Conversion error: %s(%s)\n",reason,inbuf));
 		/* smb_panic(reason); */
 		TALLOC_FREE(ob);
 		return false;
-- 
2.1.0


From ccbdedddb8c62441ddcd74485667831cf5762a1d Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Wed, 3 Jun 2015 17:07:46 +0200
Subject: [PATCH 2/2] smbd/trans2: add a useful diagnostic for files with bad
 encoding

Catch conversion error and log the path of the offending file.

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source3/smbd/trans2.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c
index 6c77e85..e518c7b 100644
--- a/source3/smbd/trans2.c
+++ b/source3/smbd/trans2.c
@@ -2374,6 +2374,10 @@ NTSTATUS smbd_dirptr_lanman2_entry(TALLOC_CTX *ctx,
 				     ppdata,
 				     end_data,
 				     &last_entry_off);
+	if (NT_STATUS_EQUAL(status, NT_STATUS_ILLEGAL_CHARACTER)) {
+		DEBUG(1,("Conversion error: illegal character: %s\n",
+			 smb_fname_str_dbg(smb_fname)));
+	}
 	TALLOC_FREE(fname);
 	TALLOC_FREE(smb_fname);
 	if (NT_STATUS_EQUAL(status, STATUS_MORE_ENTRIES)) {
-- 
2.1.0



More information about the samba-technical mailing list