[PATCH] Add substitutions %t, %j, and %J as path-safe variants of %T, %i, and %I.

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] Add substitutions %t, %j, and %J as path-safe variants of %T, %i, and %I.

Samba - samba-technical mailing list
Rationale: Using the existing substitutions in construction of paths
(dynamic shares, created on client connect) results in directory names with
colons and dots in them. Those can be hard to use when accessed from a
different share, as Windows does not allow : in paths and has some ideas about
dots.

Signed-off-by: Dr. Thomas Orgis <[hidden email]>
---
 docs-xml/manpages/smb.conf.5.xml | 19 +++++++++++++++++++
 lib/util/time.c                  | 41 ++++++++++++++++++++++++++++++++++++++++
 lib/util/time.h                  | 15 +++++++++++++++
 source3/lib/substitute.c         | 32 +++++++++++++++++++++++++++++++
 4 files changed, 107 insertions(+)

diff --git a/docs-xml/manpages/smb.conf.5.xml b/docs-xml/manpages/smb.conf.5.xml
index 49928134f09..f732c308f64 100644
--- a/docs-xml/manpages/smb.conf.5.xml
+++ b/docs-xml/manpages/smb.conf.5.xml
@@ -509,6 +509,13 @@ chmod 1770 /usr/local/samba/lib/usershares
  </listitem>
  </varlistentry>
 
+ <varlistentry>
+ <term>%J</term>
+ <listitem><para>the IP address of the client machine,
+ colons/dots replaced by underscores.</para>
+ </listitem>
+ </varlistentry>
+
  <varlistentry>
  <term>%i</term>
  <listitem><para>the local IP address to which a client connected.</para>
@@ -517,11 +524,23 @@ chmod 1770 /usr/local/samba/lib/usershares
  </listitem>
  </varlistentry>
 
+ <varlistentry>
+ <term>%j</term>
+ <listitem><para>the local IP address to which a client connected,
+ colons/dots replaced by underscores.</para>
+ </listitem>
+ </varlistentry>
+
  <varlistentry>
  <term>%T</term>
  <listitem><para>the current date and time.</para></listitem>
  </varlistentry>
 
+ <varlistentry>
+ <term>%t</term>
+ <listitem><para>the current date and time in a minimal format without colons (YYYYYmmdd_HHMMSS).</para></listitem>
+ </varlistentry>
+
  <varlistentry>
  <term>%D</term>
  <listitem><para>name of the domain or workgroup of the current user.</para></listitem>
diff --git a/lib/util/time.c b/lib/util/time.c
index 8a4d93d4ac1..9880cff0be0 100644
--- a/lib/util/time.c
+++ b/lib/util/time.c
@@ -367,6 +367,47 @@ char *current_timestring(TALLOC_CTX *ctx, bool hires)
  return timeval_string(ctx, &tv, hires);
 }
 
+/**
+ Return date and time as a minimal string avoiding funny characters
+ that may cause trouble in file names. We only use digits and
+ underscore ... or a minus/hyphen if we got negative time.
+**/
+char *minimal_timeval_string(TALLOC_CTX *ctx, const struct timeval *tp, bool hires)
+{
+ time_t t;
+ struct tm *tm;
+
+ t = (time_t)tp->tv_sec;
+ tm = localtime(&t);
+ if (!tm) {
+ if (hires) {
+ return talloc_asprintf(ctx, "%ld_%06ld",
+       (long)tp->tv_sec,
+       (long)tp->tv_usec);
+ } else {
+ return talloc_asprintf(ctx, "%ld", (long)t);
+ }
+ } else {
+ if (hires) {
+ return talloc_asprintf(ctx, "%04d%02d%02d_%02d%02d%02d_%06ld",
+       tm->tm_year+1900, tm->tm_mon+1, tm->tm_mday,
+       tm->tm_hour, tm->tm_min, tm->tm_sec,
+       (long)tp->tv_usec);
+ } else {
+ return talloc_asprintf(ctx, "%04d%02d%02d_%02d%02d%02d",
+       tm->tm_year+1900, tm->tm_mon+1, tm->tm_mday,
+       tm->tm_hour, tm->tm_min, tm->tm_sec);
+ }
+ }
+}
+
+char *current_minimal_timestring(TALLOC_CTX *ctx, bool hires)
+{
+ struct timeval tv;
+
+ GetTimeOfDay(&tv);
+ return minimal_timeval_string(ctx, &tv, hires);
+}
 
 /**
 return a HTTP/1.0 time string
diff --git a/lib/util/time.h b/lib/util/time.h
index 42d23865b82..9b897eb4b6c 100644
--- a/lib/util/time.h
+++ b/lib/util/time.h
@@ -138,6 +138,21 @@ char *timeval_string(TALLOC_CTX *ctx, const struct timeval *tp, bool hires);
 **/
 char *current_timestring(TALLOC_CTX *ctx, bool hires);
 
+/**
+ Return a date and time as a string (optionally with microseconds)
+
+ format is %Y%m%d_%H%M%S or %Y%m%d_%H%M%S_%us
+**/
+
+char *minimal_timeval_string(TALLOC_CTX *ctx, const struct timeval *tp, bool hires);
+
+/**
+ Return the current date and time as a string (optionally with microseconds)
+
+ format is %Y%m%d_%H%M%S or %Y%m%d_%H%M%S_%us
+**/
+char *current_minimal_timestring(TALLOC_CTX *ctx, bool hires);
+
 /**
 return a HTTP/1.0 time string
 **/
diff --git a/source3/lib/substitute.c b/source3/lib/substitute.c
index bc34c317974..9fdc5ca1edc 100644
--- a/source3/lib/substitute.c
+++ b/source3/lib/substitute.c
@@ -454,6 +454,20 @@ void standard_sub_basic(const char *smb_name, const char *domain_name,
  TALLOC_FREE( s );
 }
 
+/*
+ * Limit addresses to hexalpha charactes and underscore, safe for path
+ * components for Windows clients.
+ */
+static void make_address_pathsafe(char *addr)
+{
+ while(addr && *addr) {
+ if(!isxdigit(*addr)) {
+ *addr = '_';
+ }
+ ++addr;
+ }
+}
+
 /****************************************************************************
  Do some standard substitutions in a string.
  This function will return a talloced string that has to be freed.
@@ -550,11 +564,25 @@ char *talloc_sub_basic(TALLOC_CTX *mem_ctx,
  sub_peeraddr[0] ? sub_peeraddr : "0.0.0.0");
  break;
  }
+ case 'J' : {
+ r = talloc_strdup(tmp_ctx,
+ sub_peeraddr[0] ? sub_peeraddr : "0.0.0.0");
+ make_address_pathsafe(r);
+ a_string = realloc_string_sub(a_string, "%J", r);
+ break;
+ }
  case 'i':
  a_string = realloc_string_sub(
  a_string, "%i",
  sub_sockaddr[0] ? sub_sockaddr : "0.0.0.0");
  break;
+ case 'j' : {
+ r = talloc_strdup(tmp_ctx,
+ sub_sockaddr[0] ? sub_sockaddr : "0.0.0.0");
+ make_address_pathsafe(r);
+ a_string = realloc_string_sub(a_string, "%j", r);
+ break;
+ }
  case 'L' :
  if ( strncasecmp_m(p, "%LOGONSERVER%", strlen("%LOGONSERVER%")) == 0 ) {
  break;
@@ -578,6 +606,10 @@ char *talloc_sub_basic(TALLOC_CTX *mem_ctx,
  case 'T' :
  a_string = realloc_string_sub(a_string, "%T", current_timestring(tmp_ctx, False));
  break;
+ case 't' :
+ a_string = realloc_string_sub(a_string, "%t",
+      current_minimal_timestring(tmp_ctx, False));
+ break;
  case 'a' :
  a_string = realloc_string_sub(a_string, "%a",
  get_remote_arch_str());
--
2.15.0


Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Add substitutions %t, %j, and %J as path-safe variants of %T, %i, and %I.

Samba - samba-technical mailing list
On Thu, 2017-11-23 at 20:07 +0100, Dr. Thomas Orgis via samba-technical
wrote:
> Rationale: Using the existing substitutions in construction of paths
> (dynamic shares, created on client connect) results in directory names with
> colons and dots in them. Those can be hard to use when accessed from a
> different share, as Windows does not allow : in paths and has some ideas about
> dots.

G'Day Thomas,

This is an interesting addition.  Could you add some tests for it?

Here are some potential tests to extend:

source3/torture/torture.c:run_local_substitute()

source3/script/tests/test_substitutions.sh
(which uses the sub_dug and sub_dug2 shares defined in
selftest/target/Samba3.pm)

Thanks,

Andrew Bartlett
--
Andrew Bartlett
https://samba.org/~abartlet/
Authentication Developer, Samba Team         https://samba.org
Samba Development and Support, Catalyst IT  
https://catalyst.net.nz/services/samba





Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Add substitutions %t, %j, and %J as path-safe variants of %T, %i, and %I.

Samba - samba-technical mailing list
Am Fri, 24 Nov 2017 09:43:15 +1300
schrieb Andrew Bartlett via samba-technical <[hidden email]>:

> This is an interesting addition.  Could you add some tests for it?

I'm trying. I started by adding

  ok &= subst_test("%j %J", "", "", -1, -1, "0_0_0_0 0_0_0_0");

to run_local_substitute. A test for the %t substitution is more tricky
as the test for the existing %T is missing. Need to insert a fake time,
too. I guess fixing that would be the task of a patch before my patch …

Actually, I also was unable to find a reference to %T, %i or %I in the
selftest/target/Samba3.pm definitions. I would happily have added
entries for my path-safe variants, but my cargo cult approach does not
work when I do not have the fruits prepared for picking. I need to
guess a lot to get the tests right, although it should be fairly simple
for someone used to the test suite.

I am adding tests now for my variants of the substitutions that also
had tests before, and am omitting tests for the variants of
substitutions that also had no test before … this seems about fair;-)

Another patch should introduce tests for %T and %t, IMHO. An updated
version of the patch is attached. The little test I added seems to pass:

shell$ make test TESTS=local-substitute
[…]
skipping subunit (testscenarios not available)
OPTIONS --configfile=$SMB_CONF_PATH --option='fss:sequence timeout=1' --maximum-runtime=$SELFTEST_MAXTIME --basedir=$SELFTEST_TMPDIR --format=subunit --option=torture:progress=no
[1(0)/1 at 0s] samba3.smbtorture_s3.LOCAL-SUBSTITUTE

ALL OK (1 tests in 1 testsuites)

A summary with detailed information can be found in:
  ./st/summary
TOP 10 slowest tests
samba3.smbtorture_s3.LOCAL-SUBSTITUTE -> 0
'testonly' finished successfully (0.848s)
shell$


Regards,

Thomas


PS: My development box cannot run the tests successfully because
I do not have PAM (yes, still possible;-). I can configure samba
--without-pam, but the test suite is not prepared for that:

test: running (/usr/bin/perl /stuff/src/samba/selftest/selftest.pl --target=samba --prefix=./st --srcdir=/stuff/src/samba --exclude=/stuff/src/samba/selftest/skip --testlist="/usr/bin/python /stuff/src/samba/selftest/tests.py|" --testlist="/usr/bin/python /stuff/src/samba/source3/selftest/tests.py|" --testlist="/usr/bin/python /stuff/src/samba/source4/selftest/tests.py|"  --exclude=/stuff/src/samba/selftest/slow --nss_wrapper_so_path=/stuff/src/samba/bin/default/lib/nss_wrapper/libnss-wrapper.so --resolv_wrapper_so_path=/stuff/src/samba/bin/default/lib/resolv_wrapper/libresolv-wrapper.so --socket_wrapper_so_path=/stuff/src/samba/bin/default/lib/socket_wrapper/libsocket-wrapper.so --uid_wrapper_so_path=/stuff/src/samba/bin/default/lib/uid_wrapper/libuid-wrapper.so --use-dns-faking --socket-wrapper  && touch ./st/st_done) | /usr/bin/python -u /stuff/src/samba/selftest/filter-subunit --expected-failures=/stuff/src/samba/selftest/knownfail --expected-failures=/stuff/src/samba/selftest/knownfail.d --flapping=/stuff/src/samba/selftest/flapping --flapping=/stuff/src/samba/selftest/flapping.d | tee ./st/subunit | /usr/bin/python -u /stuff/src/samba/selftest/format-subunit --prefix=./st --immediate
LD_PRELOAD=/stuff/src/samba/bin/default/lib/nss_wrapper/libnss-wrapper.so:/stuff/src/samba/bin/default/lib/resolv_wrapper/libresolv-wrapper.so:/stuff/src/samba/bin/default/lib/socket_wrapper/libsocket-wrapper.so:/stuff/src/samba/bin/default/lib/uid_wrapper/libuid-wrapper.so
SOCKET_WRAPPER_DIR=/stuff/src/samba/st/w
DNS: Faking nameserver
Traceback (most recent call last):
  File "/stuff/src/samba/selftest/tests.py", line 43, in <module>
    pam_wrapper_so_path=config_hash["LIBPAM_WRAPPER_SO_PATH"]
KeyError: 'LIBPAM_WRAPPER_SO_PATH'
Error creating recipe from /usr/bin/python /stuff/src/samba/selftest/tests.py| at /stuff/src/samba/selftest/selftest.pl line 674.

ALL OK (0 tests in 0 testsuites)

A summary with detailed information can be found in:
  ./st/summary
TOP 10 slowest tests
ERROR: test command failed to complete
make: *** [Makefile:17: test] Fehler 1
16:46|host:samba$ cat st/summary
= Skipped tests =

One might catch this at the configure stage instead.

I finally resorted to the deployment machine where there is a more
standard Linux system. The full test suite has this result:

FAILED (139 failures, 33 errors and 2 unexpected successes in 48 testsuites)

I am not sure how bad or good that is. It doesn't say how many
individual tests passed to put the 139 failures into perspective. This
is a stock Ubuntu 16.04 x86-64 system. Just the samba packages on it
are patched, obviously. The server works, so it doesn't seem to be too
bad.

--
Dr. Thomas Orgis
Universität Hamburg
RRZ / Basis-Infrastruktur / HPC
Schlüterstr. 70
20146 Hamburg
Tel.: 040/42838 8826
Fax: 040/428 38 6270

0001-Add-substitutions-t-j-and-J-as-path-safe-variants-of.patch (7K) Download Attachment
smime.p7s (6K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Add substitutions %t, %j, and %J as path-safe variants of %T, %i, and %I.

Samba - samba-technical mailing list
On Mon, 2017-11-27 at 15:34 +0100, Dr. Thomas Orgis via samba-technical
wrote:

> Am Fri, 24 Nov 2017 09:43:15 +1300
> schrieb Andrew Bartlett via samba-technical <[hidden email]>:
>
> > This is an interesting addition.  Could you add some tests for it?
>
> I'm trying. I started by adding
>
>   ok &= subst_test("%j %J", "", "", -1, -1, "0_0_0_0 0_0_0_0");
>
> to run_local_substitute. A test for the %t substitution is more tricky
> as the test for the existing %T is missing. Need to insert a fake time,
> too. I guess fixing that would be the task of a patch before my patch …
>
> Actually, I also was unable to find a reference to %T, %i or %I in the
> selftest/target/Samba3.pm definitions. I would happily have added
> entries for my path-safe variants, but my cargo cult approach does not
> work when I do not have the fruits prepared for picking. I need to
> guess a lot to get the tests right, although it should be fairly simple
> for someone used to the test suite.
>
> I am adding tests now for my variants of the substitutions that also
> had tests before, and am omitting tests for the variants of
> substitutions that also had no test before … this seems about fair;-)

I know it seems fair.  However we really do need tests, otherwise we
never move forward from our current state where not enough is tested.

> Another patch should introduce tests for %T and %t, IMHO. An updated
> version of the patch is attached. The little test I added seems to pass:

Some kind of sub test for %T and %t would make this much easier to
merge.

> shell$ make test TESTS=local-substitute
> […]
> skipping subunit (testscenarios not available)
> OPTIONS --configfile=$SMB_CONF_PATH --option='fss:sequence timeout=1' --maximum-runtime=$SELFTEST_MAXTIME --basedir=$SELFTEST_TMPDIR --format=subunit --option=torture:progress=no
> [1(0)/1 at 0s] samba3.smbtorture_s3.LOCAL-SUBSTITUTE
>
> ALL OK (1 tests in 1 testsuites)
>
> A summary with detailed information can be found in:
>   ./st/summary
> TOP 10 slowest tests
> samba3.smbtorture_s3.LOCAL-SUBSTITUTE -> 0
> 'testonly' finished successfully (0.848s)
> shell$
>
>
> Regards,
>
> Thomas
>
>
> PS: My development box cannot run the tests successfully because
> I do not have PAM (yes, still possible;-). I can configure samba
> --without-pam, but the test suite is not prepared for that:
>
> test: running (/usr/bin/perl /stuff/src/samba/selftest/selftest.pl --target=samba --prefix=./st --srcdir=/stuff/src/samba --exclude=/stuff/src/samba/selftest/skip --testlist="/usr/bin/python /stuff/src/samba/selftest/tests.py|" --testlist="/usr/bin/python /stuff/src/samba/source3/selftest/tests.py|" --testlist="/usr/bin/python /stuff/src/samba/source4/selftest/tests.py|"  --exclude=/stuff/src/samba/selftest/slow --nss_wrapper_so_path=/stuff/src/samba/bin/default/lib/nss_wrapper/libnss-wrapper.so --resolv_wrapper_so_path=/stuff/src/samba/bin/default/lib/resolv_wrapper/libresolv-wrapper.so --socket_wrapper_so_path=/stuff/src/samba/bin/default/lib/socket_wrapper/libsocket-wrapper.so --uid_wrapper_so_path=/stuff/src/samba/bin/default/lib/uid_wrapper/libuid-wrapper.so --use-dns-faking --socket-wrapper  && touch ./st/st_done) | /usr/bin/python -u /stuff/src/samba/selftest/filter-subunit --expected-failures=/stuff/src/samba/selftest/knownfail --expected-failures=/stuff/src/samba/selftest/knownfail.d --flapping=/stuff/src/samba/selftest/flapping --flapping=/stuff/src/samba/selftest/flapping.d | tee ./st/subunit | /usr/bin/python -u /stuff/src/samba/selftest/format-subunit --prefix=./st --immediate
> LD_PRELOAD=/stuff/src/samba/bin/default/lib/nss_wrapper/libnss-wrapper.so:/stuff/src/samba/bin/default/lib/resolv_wrapper/libresolv-wrapper.so:/stuff/src/samba/bin/default/lib/socket_wrapper/libsocket-wrapper.so:/stuff/src/samba/bin/default/lib/uid_wrapper/libuid-wrapper.so
> SOCKET_WRAPPER_DIR=/stuff/src/samba/st/w
> DNS: Faking nameserver
> Traceback (most recent call last):
>   File "/stuff/src/samba/selftest/tests.py", line 43, in <module>
>     pam_wrapper_so_path=config_hash["LIBPAM_WRAPPER_SO_PATH"]
> KeyError: 'LIBPAM_WRAPPER_SO_PATH'
> Error creating recipe from /usr/bin/python /stuff/src/samba/selftest/tests.py| at /stuff/src/samba/selftest/selftest.pl line 674.
>
> ALL OK (0 tests in 0 testsuites)
>
> A summary with detailed information can be found in:
>   ./st/summary
> TOP 10 slowest tests
> ERROR: test command failed to complete
> make: *** [Makefile:17: test] Fehler 1
> 16:46|host:samba$ cat st/summary
> = Skipped tests =
>
> One might catch this at the configure stage instead.

I agree this isn't ideal.  The fix is harder, if we make the test more
flexible we risk accidentally cutting out the testsuite when a
configure test breaks (fails to detect PAM for example), but if we make
the configure time checks stricter then we won't run at all except in
the 'blessed' environment, making deployments beyond that less tested.

> I finally resorted to the deployment machine where there is a more
> standard Linux system. The full test suite has this result:
>
> FAILED (139 failures, 33 errors and 2 unexpected successes in 48 testsuites)

That doesn't look like enough testsuites.

> I am not sure how bad or good that is. It doesn't say how many
> individual tests passed to put the 139 failures into perspective. This
> is a stock Ubuntu 16.04 x86-64 system. Just the samba packages on it
> are patched, obviously. The server works, so it doesn't seem to be too
> bad.

We are working to get more cloud-based testing available to make this
easier to contribute to, which will be in known-good environments.

Andrew Bartlett

--
Andrew Bartlett
https://samba.org/~abartlet/
Authentication Developer, Samba Team         https://samba.org
Samba Development and Support, Catalyst IT  
https://catalyst.net.nz/services/samba