[PATCH] New option "mangled names = illegal"
Ralph Böhme
slow at samba.org
Thu Dec 15 12:28:00 UTC 2016
Hi!
This one seemed obvious to me, I must be missing something. :)
With "mangled names = yes" we create mangled names for
1) any name that contains illegal NTFS characters
2) any name that exceeds 8.3 length limits
In case of 2 the mangled name is put into the shortname buffer in find
responses. Modern clients don't use the shortname anymore, so having an
additional "mangled names" option that mangles only for case 1), but not 2)
would be useful and reduce load on the server.
Attached is a patch that implements such an option, I've called it "mangled
names = illegal".
A benchmark shows that it improves performance by more then 10% in corner cases
like a directory with 20k files with long names:
$ cd /path/share/dir
$ i=1; while [ $i -lt 20001 ] ; do echo longfilename$i; touch longfilename$i; ((i++)); done
"mangled named = yes"
=====================
$ time ./bin/smbclient -U slow%x -m smb3 //localhost/share -c "ls dir\*" > /dev/null
Domain=[SLOWSERVER] OS=[] Server=[]
real 0m0.402s
user 0m0.055s
sys 0m0.036s
"mangled named = illegal"
=========================
$ time ./bin/smbclient -U slow%x -m smb3 //localhost/share -c "ls dir\*" > /dev/null
Domain=[SLOWSERVER] OS=[] Server=[]
real 0m0.352s
user 0m0.063s
sys 0m0.031s
What do you think?
Cheerio!
-slow
-------------- next part --------------
From f4767e1579fbb593971bbbb118495be108cde887 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Thu, 17 Nov 2016 14:22:41 +0100
Subject: [PATCH 1/3] s3/smbd: convert "mangled names" option to an enum
This is in preparation of adding an additional setting for this
option. No change in behaviour by this commit, that comes in the next
one.
Signed-off-by: Ralph Boehme <slow at samba.org>
---
lib/param/loadparm.h | 3 +++
lib/param/param_table.c | 10 ++++++++++
source3/param/loadparm.c | 2 +-
source3/smbd/mangle.c | 4 ++--
source3/smbd/trans2.c | 12 ++++++++++--
5 files changed, 26 insertions(+), 5 deletions(-)
diff --git a/lib/param/loadparm.h b/lib/param/loadparm.h
index f9fb7d8..2cd5cca 100644
--- a/lib/param/loadparm.h
+++ b/lib/param/loadparm.h
@@ -236,6 +236,9 @@ enum inheritowner_options {
INHERIT_OWNER_UNIX_ONLY
};
+/* mangled names options */
+enum mangled_names_options {MANGLED_NAMES_NO, MANGLED_NAMES_YES};
+
/*
* Default passwd chat script.
*/
diff --git a/lib/param/param_table.c b/lib/param/param_table.c
index 4b5234a..8eb791a 100644
--- a/lib/param/param_table.c
+++ b/lib/param/param_table.c
@@ -315,6 +315,16 @@ static const struct enum_list enum_inherit_owner_vals[] = {
{INHERIT_OWNER_UNIX_ONLY, "unix only"},
{-1, NULL}};
+static const struct enum_list enum_mangled_names[] = {
+ {MANGLED_NAMES_NO, "no"},
+ {MANGLED_NAMES_NO, "false"},
+ {MANGLED_NAMES_NO, "0"},
+ {MANGLED_NAMES_YES, "yes"},
+ {MANGLED_NAMES_YES, "true"},
+ {MANGLED_NAMES_YES, "1"},
+ {-1, NULL}
+};
+
/* Note: We do not initialise the defaults union - it is not allowed in ANSI C
*
* NOTE: Handling of duplicated (synonym) parameters:
diff --git a/source3/param/loadparm.c b/source3/param/loadparm.c
index d8da749..ac9ba49 100644
--- a/source3/param/loadparm.c
+++ b/source3/param/loadparm.c
@@ -201,7 +201,7 @@ static struct loadparm_service sDefault =
.oplocks = true,
.kernel_oplocks = false,
.level2_oplocks = true,
- .mangled_names = true,
+ .mangled_names = MANGLED_NAMES_YES,
.wide_links = false,
.follow_symlinks = true,
.sync_always = false,
diff --git a/source3/smbd/mangle.c b/source3/smbd/mangle.c
index a8988f0..3649bf7 100644
--- a/source3/smbd/mangle.c
+++ b/source3/smbd/mangle.c
@@ -104,7 +104,7 @@ bool mangle_is_8_3_wildcards(const char *fname, bool check_case,
bool mangle_must_mangle(const char *fname,
const struct share_params *p)
{
- if (!lp_mangled_names(p)) {
+ if (lp_mangled_names(p) == MANGLED_NAMES_NO) {
return False;
}
return mangle_fns->must_mangle(fname, p);
@@ -140,7 +140,7 @@ bool name_to_8_3(const char *in,
/* name mangling can be disabled for speed, in which case
we just truncate the string */
- if (!lp_mangled_names(p)) {
+ if (lp_mangled_names(p) == MANGLED_NAMES_NO) {
strlcpy(out, in, 13);
return True;
}
diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c
index 6999b2d..6fe3f92 100644
--- a/source3/smbd/trans2.c
+++ b/source3/smbd/trans2.c
@@ -2440,11 +2440,17 @@ NTSTATUS smbd_dirptr_lanman2_entry(TALLOC_CTX *ctx,
bool ok;
uint64_t last_entry_off = 0;
NTSTATUS status;
+ enum mangled_names_options mangled_names;
+ bool marshall_with_83_names;
+
+ mangled_names = lp_mangled_names(conn->params);
ZERO_STRUCT(state);
state.conn = conn;
state.info_level = info_level;
- state.check_mangled_names = lp_mangled_names(conn->params);
+ if (mangled_names != MANGLED_NAMES_NO) {
+ state.check_mangled_names = true;
+ }
state.has_wild = dptr_has_wild(dirptr);
state.got_exact_match = false;
@@ -2480,12 +2486,14 @@ NTSTATUS smbd_dirptr_lanman2_entry(TALLOC_CTX *ctx,
*got_exact_match = state.got_exact_match;
+ marshall_with_83_names = (mangled_names == MANGLED_NAMES_YES);
+
status = smbd_marshall_dir_entry(ctx,
conn,
flags2,
info_level,
name_list,
- state.check_mangled_names,
+ marshall_with_83_names,
requires_resume_key,
mode,
fname,
--
2.7.4
From c36e93243627a804b3dcecd1475eac51f780ea2b Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Thu, 17 Nov 2016 14:24:07 +0100
Subject: [PATCH 2/3] s3/smbd: new "mangled names" setting "illegal"
This does mangling for names with illegal NTFS characters, but not for
names longer then 8.3:
Name mangling with mangled named = yes
======================================
Mangled | Short | Name
----------------------------
| | foo
| yes | 123456789
yes | | foo:bar
Name mangling with mangled named = illegal
==========================================
Mangled | Short | Name
----------------------------
| | foo
| | 123456789
yes | | foo:bar
Setting "mangled names = illegal" is the most sensible setting for
modern clients that don't use the shortname anymore.
Signed-off-by: Ralph Boehme <slow at samba.org>
---
docs-xml/smbdotconf/filename/manglednames.xml | 20 +++++++++++++++++++-
lib/param/loadparm.h | 2 +-
lib/param/param_table.c | 1 +
3 files changed, 21 insertions(+), 2 deletions(-)
diff --git a/docs-xml/smbdotconf/filename/manglednames.xml b/docs-xml/smbdotconf/filename/manglednames.xml
index bd5d97f..972834e 100644
--- a/docs-xml/smbdotconf/filename/manglednames.xml
+++ b/docs-xml/smbdotconf/filename/manglednames.xml
@@ -1,5 +1,6 @@
<samba:parameter name="mangled names"
- type="boolean"
+ type="enum"
+ enumlist="enum_mangled_names"
context="S"
parm="1"
xmlns:samba="http://www.samba.org/samba/DTD/samba-doc">
@@ -11,6 +12,22 @@
<para>See the section on <smbconfoption name="name mangling"/> for
details on how to control the mangling process.</para>
+ <para>Possible option settings are</para>
+
+ <itemizedlist>
+ <listitem><para><emphasis>yes (default)</emphasis> -
+ enables name mangling for all not DOS 8.3 conforming
+ names.</para></listitem>
+
+ <listitem><para><emphasis>no</emphasis> - disables any
+ name mangling.</para></listitem>
+
+ <listitem><para><emphasis>illegal</emphasis> - does
+ mangling for names with illegal NTFS characters. This
+ is the most sensible setting for modern clients that
+ don't use the shortname anymore.</para></listitem>
+ </itemizedlist>
+
<para>If mangling is used then the mangling method is as follows:</para>
<itemizedlist>
@@ -56,4 +73,5 @@
do not change between sessions.</para>
</description>
<value type="default">yes</value>
+<value type="example">illegal</value>
</samba:parameter>
diff --git a/lib/param/loadparm.h b/lib/param/loadparm.h
index 2cd5cca..6d01b37 100644
--- a/lib/param/loadparm.h
+++ b/lib/param/loadparm.h
@@ -237,7 +237,7 @@ enum inheritowner_options {
};
/* mangled names options */
-enum mangled_names_options {MANGLED_NAMES_NO, MANGLED_NAMES_YES};
+enum mangled_names_options {MANGLED_NAMES_NO, MANGLED_NAMES_YES, MANGLED_NAMES_ILLEGAL};
/*
* Default passwd chat script.
diff --git a/lib/param/param_table.c b/lib/param/param_table.c
index 8eb791a..95c3b8c 100644
--- a/lib/param/param_table.c
+++ b/lib/param/param_table.c
@@ -319,6 +319,7 @@ static const struct enum_list enum_mangled_names[] = {
{MANGLED_NAMES_NO, "no"},
{MANGLED_NAMES_NO, "false"},
{MANGLED_NAMES_NO, "0"},
+ {MANGLED_NAMES_ILLEGAL, "illegal"},
{MANGLED_NAMES_YES, "yes"},
{MANGLED_NAMES_YES, "true"},
{MANGLED_NAMES_YES, "1"},
--
2.7.4
From d3714c79dee8a406f91b9af2d9f84db608cfa49b Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Thu, 15 Dec 2016 13:05:50 +0100
Subject: [PATCH 3/3] s3/torture: add a test for "mangled names = invalid"
This checks both that illegal NTFS names are still mangled and that long
names have no shortname.
Signed-off-by: Ralph Boehme <slow at samba.org>
---
selftest/target/Samba3.pm | 4 ++
source3/selftest/tests.py | 5 ++
source3/torture/torture.c | 147 ++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 156 insertions(+)
diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm
index 9013652..1d77c97 100755
--- a/selftest/target/Samba3.pm
+++ b/selftest/target/Samba3.pm
@@ -1830,6 +1830,10 @@ sub provision($$$$$$$$)
copy = tmp
acl_xattr:ignore system acls = yes
acl_xattr:default acl style = windows
+
+[mangle_illegal]
+ copy = tmp
+ mangled names = illegal
";
close(CONF);
diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py
index d9d32cc..2feb5c8 100755
--- a/source3/selftest/tests.py
+++ b/source3/selftest/tests.py
@@ -79,6 +79,11 @@ tests = ["OPLOCK-CANCEL"]
for t in tests:
plantestsuite("samba3.smbtorture_s3.plain(nt4_dc).%s" % t, "nt4_dc", [os.path.join(samba3srcdir, "script/tests/test_smbtorture_s3.sh"), t, '//$SERVER_IP/tmp', '$USERNAME', '$PASSWORD', smbtorture3, "", "-l $LOCAL_PATH"])
+env = "nt4_dc"
+tests = ["MANGLE-ILLEGAL"]
+for t in tests:
+ plantestsuite("samba3.smbtorture_s3.plain(%s).%s" % (env, t), env, [os.path.join(samba3srcdir, "script/tests/test_smbtorture_s3.sh"), t, '//$SERVER_IP/mangle_illegal', '$USERNAME', '$PASSWORD', smbtorture3, "", "-l $LOCAL_PATH"])
+
tests = ["RW1", "RW2", "RW3"]
for t in tests:
plantestsuite("samba3.smbtorture_s3.vfs_aio_fork(simpleserver).%s" % t, "simpleserver", [os.path.join(samba3srcdir, "script/tests/test_smbtorture_s3.sh"), t, '//$SERVER_IP/vfs_aio_fork', '$USERNAME', '$PASSWORD', smbtorture3, "", "-l $LOCAL_PATH"])
diff --git a/source3/torture/torture.c b/source3/torture/torture.c
index 8f0f136..4554bc4 100644
--- a/source3/torture/torture.c
+++ b/source3/torture/torture.c
@@ -8448,6 +8448,152 @@ static bool run_mangle1(int dummy)
return true;
}
+static NTSTATUS mangle_illegal_list_shortname_fn(const char *mntpoint,
+ struct file_info *f,
+ const char *mask,
+ void *state)
+{
+ if (f->short_name == NULL) {
+ return NT_STATUS_OK;
+ }
+
+ if (strlen(f->short_name) == 0) {
+ return NT_STATUS_OK;
+ }
+
+ printf("unexpected shortname: %s\n", f->short_name);
+
+ return NT_STATUS_OBJECT_NAME_INVALID;
+}
+
+static NTSTATUS mangle_illegal_list_name_fn(const char *mntpoint,
+ struct file_info *f,
+ const char *mask,
+ void *state)
+{
+ char *name = state;
+
+ printf("name: %s\n", f->name);
+ fstrcpy(name, f->name);
+ return NT_STATUS_OK;
+}
+
+static bool run_mangle_illegal(int dummy)
+{
+ struct cli_state *cli = NULL;
+ struct cli_state *cli_posix = NULL;
+ const char *fname = "\\MANGLE_ILLEGAL\\this_is_a_long_fname_to_be_mangled.txt";
+ const char *illegal_fname = "MANGLE_ILLEGAL/foo:bar";
+ char *mangled_path = NULL;
+ uint16_t fnum;
+ fstring name;
+ fstring alt_name;
+ NTSTATUS status;
+
+ printf("starting mangle-illegal test\n");
+
+ if (!torture_open_connection(&cli, 0)) {
+ return False;
+ }
+
+ smbXcli_conn_set_sockopt(cli->conn, sockops);
+
+ if (!torture_open_connection(&cli_posix, 0)) {
+ return false;
+ }
+
+ smbXcli_conn_set_sockopt(cli_posix->conn, sockops);
+
+ status = torture_setup_unix_extensions(cli_posix);
+ if (!NT_STATUS_IS_OK(status)) {
+ return false;
+ }
+
+ cli_rmdir(cli, "\\MANGLE_ILLEGAL");
+ status = cli_mkdir(cli, "\\MANGLE_ILLEGAL");
+ if (!NT_STATUS_IS_OK(status)) {
+ printf("mkdir1 failed : %s\n", nt_errstr(status));
+ return False;
+ }
+
+ /*
+ * Create a file with illegal NTFS characters and test that we
+ * get a usable mangled name
+ */
+
+ cli_setatr(cli_posix, illegal_fname, 0, 0);
+ cli_posix_unlink(cli_posix, illegal_fname);
+
+ status = cli_posix_open(cli_posix, illegal_fname, O_RDWR|O_CREAT|O_EXCL,
+ 0600, &fnum);
+ if (!NT_STATUS_IS_OK(status)) {
+ printf("POSIX create of %s failed (%s)\n",
+ illegal_fname, nt_errstr(status));
+ return false;
+ }
+
+ status = cli_close(cli_posix, fnum);
+ if (!NT_STATUS_IS_OK(status)) {
+ printf("close failed (%s)\n", nt_errstr(status));
+ return false;
+ }
+
+ status = cli_list(cli, "\\MANGLE_ILLEGAL\\*", 0, mangle_illegal_list_name_fn, &name);
+ if (!NT_STATUS_IS_OK(status)) {
+ d_printf("cli_list failed: %s\n", nt_errstr(status));
+ return false;
+ }
+
+ mangled_path = talloc_asprintf(talloc_tos(), "\\MANGLE_ILLEGAL\\%s", name);
+ if (mangled_path == NULL) {
+ return false;
+ }
+
+ status = cli_openx(cli, mangled_path, O_RDONLY, DENY_NONE, &fnum);
+ if (!NT_STATUS_IS_OK(status)) {
+ d_printf("cli_openx(%s) failed: %s\n", mangled_path, nt_errstr(status));
+ TALLOC_FREE(mangled_path);
+ return false;
+ }
+ TALLOC_FREE(mangled_path);
+ cli_close(cli, fnum);
+
+ cli_setatr(cli_posix, illegal_fname, 0, 0);
+ cli_posix_unlink(cli_posix, illegal_fname);
+
+ /*
+ * Create a file with a long name and check that we got *no* short name.
+ */
+
+ status = cli_ntcreate(cli, fname, 0, GENERIC_ALL_ACCESS|DELETE_ACCESS,
+ FILE_ATTRIBUTE_NORMAL, 0, FILE_OVERWRITE_IF,
+ 0, 0, &fnum, NULL);
+ if (!NT_STATUS_IS_OK(status)) {
+ d_printf("open %s failed: %s\n", fname, nt_errstr(status));
+ return false;
+ }
+ cli_close(cli, fnum);
+
+ status = cli_list(cli, fname, 0, mangle_illegal_list_shortname_fn, &alt_name);
+ if (!NT_STATUS_IS_OK(status)) {
+ d_printf("cli_list failed\n");
+ return false;
+ }
+
+ cli_unlink(cli, fname, 0);
+ cli_rmdir(cli, "\\MANGLE_ILLEGAL");
+
+ if (!torture_close_connection(cli_posix)) {
+ return false;
+ }
+
+ if (!torture_close_connection(cli)) {
+ return false;
+ }
+
+ return true;
+}
+
static size_t null_source(uint8_t *buf, size_t n, void *priv)
{
size_t *to_pull = (size_t *)priv;
@@ -11042,6 +11188,7 @@ static struct {
{"PROPERTIES", run_properties, 0},
{"MANGLE", torture_mangle, 0},
{"MANGLE1", run_mangle1, 0},
+ {"MANGLE-ILLEGAL", run_mangle_illegal, 0},
{"W2K", run_w2ktest, 0},
{"TRANS2SCAN", torture_trans2_scan, 0},
{"NTTRANSSCAN", torture_nttrans_scan, 0},
--
2.7.4
More information about the samba-technical
mailing list