[exim-cvs] readsocket expansion: response caching

Top Page
Delete this message
Reply to this message
Author: Exim Git Commits Mailing List
Date:  
To: exim-cvs
Subject: [exim-cvs] readsocket expansion: response caching
Gitweb: https://git.exim.org/exim.git/commitdiff/1950cf85b47f7c8407b3318a4f8fc57c0af6d6ba
Commit:     1950cf85b47f7c8407b3318a4f8fc57c0af6d6ba
Parent:     accf9211ea33262b8865805a4f61155f26320444
Author:     Jeremy Harris <jgh146exb@???>
AuthorDate: Sat Apr 18 15:36:54 2020 +0100
Committer:  Jeremy Harris <jgh146exb@???>
CommitDate: Wed Apr 22 19:27:27 2020 +0100


    readsocket expansion: response caching
---
 doc/doc-docbook/spec.xfpt    |  36 +++--
 doc/doc-txt/NewStuff         |   5 +-
 src/scripts/MakeLinks        |   4 +-
 src/scripts/lookups-Makefile |   3 +
 src/src/drtables.c           |   4 +
 src/src/expand.c             | 244 +++++++--------------------------
 src/src/functions.h          |   2 +
 src/src/lookups/Makefile     |   1 +
 src/src/lookups/readsock.c   | 319 +++++++++++++++++++++++++++++++++++++++++++
 src/src/search.c             |  22 ++-
 test/confs/0373              |   2 +
 test/log/0373                |   2 +-
 test/rejectlog/0373          |   2 +-
 test/scripts/0000-Basic/0373 |  49 +++++--
 test/stderr/2200             |   2 +-
 test/stderr/2201             |   2 +-
 test/stderr/2610             |   4 +-
 test/stderr/2620             |   4 +-
 test/stdout/0373             |  56 ++++++--
 19 files changed, 513 insertions(+), 250 deletions(-)


diff --git a/doc/doc-docbook/spec.xfpt b/doc/doc-docbook/spec.xfpt
index c544371..8702485 100644
--- a/doc/doc-docbook/spec.xfpt
+++ b/doc/doc-docbook/spec.xfpt
@@ -10285,21 +10285,37 @@ ${readsocket{/socket/name}{request string}{3s}}
.endd

The third argument is a list of options, of which the first element is the timeout
-and must be present if the argument is given.
+and must be present if any options are given.
Further elements are options of form &'name=value'&.
-Two option types is currently recognised: shutdown and tls.
-The first defines whether (the default)
-or not a shutdown is done on the connection after sending the request.
-Example, to not do so (preferred, eg. by some webservers):
+Example:
.code
${readsocket{/socket/name}{request string}{3s:shutdown=no}}
.endd
-The second, tls, controls the use of TLS on the connection. Example:
-.code
-${readsocket{/socket/name}{request string}{3s:tls=yes}}
-.endd
-The default is to not use TLS.
+
+.new
+The following option names are recognised:
+.ilist
+&*cache*&
+Defines if the result data can be cached for use by a later identical
+request in the same process.
+Values are &"yes"& or &"no"& (the default).
+If not, all cached results for this connection specification
+will be invalidated.
+
+.next
+&*shutdown*&
+Defines whether or not a write-shutdown is done on the connection after
+sending the request. Values are &"yes"& (the default) or &"no"&
+(preferred, eg. by some webservers).
+
+.next
+&*tls*&
+Controls the use of TLS on the connection.
+Values are &"yes"& or &"no"& (the default).
If it is enabled, a shutdown as descripbed above is never done.
+.endlist
+.wen
+

 A fourth argument allows you to change any newlines that are in the data
 that is read, in the same way as for &%readfile%& (see above). This example
diff --git a/doc/doc-txt/NewStuff b/doc/doc-txt/NewStuff
index f922f2c..4ae49c2 100644
--- a/doc/doc-txt/NewStuff
+++ b/doc/doc-txt/NewStuff
@@ -54,7 +54,10 @@ Version 4.94
 14. Options on pgsql and mysql lookups, to specify server separate from the
     lookup string.


-15. Expansion item ${listquote {<char} {<item>}}
+15. Expansion item ${listquote {<char} {<item>}}.
+
+16. An option for the ${readsocket {}{}{}} expansion to make the result data
+    cacheable.




diff --git a/src/scripts/MakeLinks b/src/scripts/MakeLinks
index 14fdb00..277a1c0 100755
--- a/src/scripts/MakeLinks
+++ b/src/scripts/MakeLinks
@@ -31,8 +31,8 @@ mkdir lookups
cd lookups
# Makefile is generated
for f in README cdb.c dbmdb.c dnsdb.c dsearch.c ibase.c json.c ldap.h ldap.c \
- lmdb.c lsearch.c mysql.c redis.c nis.c nisplus.c oracle.c passwd.c \
- pgsql.c spf.c sqlite.c testdb.c whoson.c \
+ lmdb.c lsearch.c mysql.c nis.c nisplus.c oracle.c passwd.c \
+ pgsql.c readsock.c redis.c spf.c sqlite.c testdb.c whoson.c \
lf_functions.h lf_check_file.c lf_quote.c lf_sqlperform.c
do
ln -s ../../src/lookups/$f $f
diff --git a/src/scripts/lookups-Makefile b/src/scripts/lookups-Makefile
index 498184f..63f3eb8 100755
--- a/src/scripts/lookups-Makefile
+++ b/src/scripts/lookups-Makefile
@@ -182,6 +182,9 @@ then
OBJ="${OBJ} lmdb.o"
fi

+# readsock is always wanted as it implements the ${readsock } expansion
+OBJ="${OBJ} readsock.o"
+
echo "MODS = $MODS"
echo "OBJ = $OBJ"

diff --git a/src/src/drtables.c b/src/src/drtables.c
index 363c07b..7ecca50 100644
--- a/src/src/drtables.c
+++ b/src/src/drtables.c
@@ -617,6 +617,8 @@ extern lookup_module_info testdb_lookup_module_info;
extern lookup_module_info whoson_lookup_module_info;
#endif

+extern lookup_module_info readsock_lookup_module_info;
+

void
init_lookup_list(void)
@@ -715,6 +717,8 @@ addlookupmodule(NULL, &testdb_lookup_module_info);
addlookupmodule(NULL, &whoson_lookup_module_info);
#endif

+addlookupmodule(NULL, &readsock_lookup_module_info);
+
 #ifdef LOOKUP_MODULE_DIR
 if (!(dd = exim_opendir(LOOKUP_MODULE_DIR)))
   {
diff --git a/src/src/expand.c b/src/src/expand.c
index cdc914f..5ae74ef 100644
--- a/src/src/expand.c
+++ b/src/src/expand.c
@@ -3927,7 +3927,7 @@ Arguments:
 Returns:       new pointer for expandable string, terminated if non-null
 */


-static gstring *
+gstring *
cat_file(FILE *f, gstring *yield, uschar *eol)
{
uschar buffer[1024];
@@ -3947,7 +3947,7 @@ return yield;


 #ifndef DISABLE_TLS
-static gstring *
+gstring *
 cat_file_tls(void * tls_ctx, gstring * yield, uschar * eol)
 {
 int rc;
@@ -5286,7 +5286,6 @@ while (*s != 0)
       host_item host;
       BOOL do_shutdown = TRUE;
       BOOL do_tls = FALSE;    /* Only set under ! DISABLE_TLS */
-      blob reqstr;


       if (expand_forbid & RDO_READSOCK)
         {
@@ -5304,218 +5303,77 @@ while (*s != 0)
         case 3: goto EXPAND_FAILED;
         }


-      /* Grab the request string, if any */
-
-      reqstr.data = sub_arg[1];
-      reqstr.len = Ustrlen(sub_arg[1]);
-
-      /* Sort out timeout, if given.  The second arg is a list with the first element
-      being a time value.  Any more are options of form "name=value".  Currently the
-      only options recognised are "shutdown" and "tls". */
-
-      if (sub_arg[2])
-        {
-    const uschar * list = sub_arg[2];
-    uschar * item;
-    int sep = 0;
-
-    if (  !(item = string_nextinlist(&list, &sep, NULL, 0))
-       || !*item
-       || (timeout = readconf_readtime(item, 0, FALSE)) < 0)
-          {
-          expand_string_message = string_sprintf("bad time value %s", item);
-          goto EXPAND_FAILED;
-          }
-
-    while ((item = string_nextinlist(&list, &sep, NULL, 0)))
-      if (Ustrncmp(item, US"shutdown=", 9) == 0)
-        { if (Ustrcmp(item + 9, US"no") == 0) do_shutdown = FALSE; }
-#ifndef DISABLE_TLS
-      else if (Ustrncmp(item, US"tls=", 4) == 0)
-        { if (Ustrcmp(item + 9, US"no") != 0) do_tls = TRUE; }
-#endif
-        }
-      else
-    sub_arg[3] = NULL;                     /* No eol if no timeout */
-
       /* If skipping, we don't actually do anything. Otherwise, arrange to
       connect to either an IP or a Unix socket. */


       if (!skipping)
         {
-        /* Handle an IP (internet) domain */
-
-        if (Ustrncmp(sub_arg[0], "inet:", 5) == 0)
-          {
-          int port;
-          uschar * port_name;
+    int stype = search_findtype(US"readsock", 8);
+    gstring * g = NULL;
+    void * handle;
+    int expand_setup = -1;
+    uschar * s;


-          server_name = sub_arg[0] + 5;
-          port_name = Ustrrchr(server_name, ':');
+    /* If the reqstr is empty, flag that and set a dummy */


-          /* Sort out the port */
+    if (!sub_arg[1][0])
+      {
+      g = string_append_listele(g, ',', US"send=no");
+      sub_arg[1] = US"DUMMY";
+      }


-          if (!port_name)
-            {
-            expand_string_message =
-              string_sprintf("missing port for readsocket %s", sub_arg[0]);
-            goto EXPAND_FAILED;
-            }
-          *port_name++ = 0;           /* Terminate server name */
+    /* Re-marshall the options */


-          if (isdigit(*port_name))
-            {
-            uschar *end;
-            port = Ustrtol(port_name, &end, 0);
-            if (end != port_name + Ustrlen(port_name))
-              {
-              expand_string_message =
-                string_sprintf("invalid port number %s", port_name);
-              goto EXPAND_FAILED;
-              }
-            }
-          else
-            {
-            struct servent *service_info = getservbyname(CS port_name, "tcp");
-            if (!service_info)
-              {
-              expand_string_message = string_sprintf("unknown port \"%s\"",
-                port_name);
-              goto EXPAND_FAILED;
-              }
-            port = ntohs(service_info->s_port);
-            }
+    if (sub_arg[2])
+      {
+      const uschar * list = sub_arg[2];
+      uschar * item;
+      int sep = 0;


-      /*XXX we trust that the request is idempotent for TFO.  Hmm. */
-      cctx.sock = ip_connectedsocket(SOCK_STREAM, server_name, port, port,
-          timeout, &host, &expand_string_message,
-          do_tls ? NULL : &reqstr);
-      callout_address = NULL;
-      if (cctx.sock < 0)
-        goto SOCK_FAIL;
-      if (!do_tls)
-        reqstr.len = 0;
-          }
+      /* First option has no tag and is timeout */
+      if ((item = string_nextinlist(&list, &sep, NULL, 0)))
+        g = string_append_listele(g, ',',
+          string_sprintf("timeout=%s", item));


-        /* Handle a Unix domain socket */
+      /* The rest of the options from the expansion */
+      while ((item = string_nextinlist(&list, &sep, NULL, 0)))
+        g = string_append_listele(g, ',', item);


-        else
-          {
-      struct sockaddr_un sockun;         /* don't call this "sun" ! */
-          int rc;
+      /* possibly plus an EOL string */
+      if (sub_arg[3] && *sub_arg[3])
+        g = string_append_listele(g, ',',
+          string_sprintf("eol=%s", sub_arg[3]));


-          if ((cctx.sock = socket(PF_UNIX, SOCK_STREAM, 0)) == -1)
-            {
-            expand_string_message = string_sprintf("failed to create socket: %s",
-              strerror(errno));
-            goto SOCK_FAIL;
-            }
-
-          sockun.sun_family = AF_UNIX;
-          sprintf(sockun.sun_path, "%.*s", (int)(sizeof(sockun.sun_path)-1),
-            sub_arg[0]);
-      server_name = US sockun.sun_path;
-
-          sigalrm_seen = FALSE;
-          ALARM(timeout);
-          rc = connect(cctx.sock, (struct sockaddr *)(&sockun), sizeof(sockun));
-          ALARM_CLR(0);
-          if (sigalrm_seen)
-            {
-            expand_string_message = US "socket connect timed out";
-            goto SOCK_FAIL;
-            }
-          if (rc < 0)
-            {
-            expand_string_message = string_sprintf("failed to connect to socket "
-              "%s: %s", sub_arg[0], strerror(errno));
-            goto SOCK_FAIL;
-            }
-      host.name = server_name;
-      host.address = US"";
-          }
+      }


-        DEBUG(D_expand) debug_printf_indent("connected to socket %s\n", sub_arg[0]);
+    /* Gat a (possibly cached) handle for the connection */


-#ifndef DISABLE_TLS
-    if (do_tls)
+    if (!(handle = search_open(sub_arg[0], stype, 0, NULL, NULL)))
       {
-      smtp_connect_args conn_args = {.host = &host };
-      tls_support tls_dummy = {.sni=NULL};
-      uschar * errstr;
-
-      if (!tls_client_start(&cctx, &conn_args, NULL, &tls_dummy, &errstr))
-        {
-        expand_string_message = string_sprintf("TLS connect failed: %s", errstr);
-        goto SOCK_FAIL;
-        }
+      if (*expand_string_message) goto EXPAND_FAILED;
+      expand_string_message = search_error_message;
+      search_error_message = NULL;
+      goto SOCK_FAIL;
       }
-#endif
-
-    /* Allow sequencing of test actions */
-    testharness_pause_ms(100);
-
-        /* Write the request string, if not empty or already done */
-
-        if (reqstr.len)
-          {
-          DEBUG(D_expand) debug_printf_indent("writing \"%s\" to socket\n",
-            reqstr.data);
-          if ( (
-#ifndef DISABLE_TLS
-          do_tls ? tls_write(cctx.tls_ctx, reqstr.data, reqstr.len, FALSE) :
-#endif
-            write(cctx.sock, reqstr.data, reqstr.len)) != reqstr.len)
-            {
-            expand_string_message = string_sprintf("request write to socket "
-              "failed: %s", strerror(errno));
-            goto SOCK_FAIL;
-            }
-          }
-
-        /* Shut down the sending side of the socket. This helps some servers to
-        recognise that it is their turn to do some work. Just in case some
-        system doesn't have this function, make it conditional. */


-#ifdef SHUT_WR
-    if (!do_tls && do_shutdown) shutdown(cctx.sock, SHUT_WR);
-#endif
-
-    testharness_pause_ms(100);
-
-        /* Now we need to read from the socket, under a timeout. The function
-        that reads a file can be used. */
-
-    if (!do_tls)
-      fp = fdopen(cctx.sock, "rb");
-        sigalrm_seen = FALSE;
-        ALARM(timeout);
-        yield =
-#ifndef DISABLE_TLS
-      do_tls ? cat_file_tls(cctx.tls_ctx, yield, sub_arg[3]) :
-#endif
-            cat_file(fp, yield, sub_arg[3]);
-        ALARM_CLR(0);
+    /* Get (possibly cached) results for the lookup */
+    /* sspec: sub_arg[0]  req: sub_arg[1]  opts: g */


-#ifndef DISABLE_TLS
-    if (do_tls)
+    if ((s = search_find(handle, sub_arg[0], sub_arg[1], -1, NULL, 0, 0,
+                    &expand_setup, string_from_gstring(g))))
+      yield = string_cat(yield, s);
+    else if (f.search_find_defer)
       {
-      tls_close(cctx.tls_ctx, TRUE);
-      close(cctx.sock);
+      expand_string_message = search_error_message;
+      search_error_message = NULL;
+      goto SOCK_FAIL;
       }
     else
-#endif
-      (void)fclose(fp);
-
-        /* After a timeout, we restore the pointer in the result, that is,
-        make sure we add nothing from the socket. */
-
-        if (sigalrm_seen)
-          {
-          if (yield) yield->ptr = save_ptr;
-          expand_string_message = US "socket read timed out";
-          goto SOCK_FAIL;
-          }
+      {    /* should not happen, at present */
+      expand_string_message = search_error_message;
+      search_error_message = NULL;
+      goto SOCK_FAIL;
+      }
         }


       /* The whole thing has worked (or we were skipping). If there is a
diff --git a/src/src/functions.h b/src/src/functions.h
index 8206c48..8ea5e4e 100644
--- a/src/src/functions.h
+++ b/src/src/functions.h
@@ -149,6 +149,8 @@ extern void    bits_clear(unsigned int *, size_t, int *);
 extern void    bits_set(unsigned int *, size_t, int *);


 extern void    cancel_cutthrough_connection(BOOL, const uschar *);
+extern gstring *cat_file(FILE *, gstring *, uschar *);
+extern gstring *cat_file_tls(void *, gstring *, uschar *);
 extern int     check_host(void *, const uschar *, const uschar **, uschar **);
 extern uschar **child_exec_exim(int, BOOL, int *, BOOL, int, ...);
 extern pid_t   child_open_exim_function(int *, const uschar *);
diff --git a/src/src/lookups/Makefile b/src/src/lookups/Makefile
index 01910d5..9231d66 100644
--- a/src/src/lookups/Makefile
+++ b/src/src/lookups/Makefile
@@ -43,6 +43,7 @@ nisplus.o:       $(PHDRS) nisplus.c
 oracle.o:        $(PHDRS) oracle.c
 passwd.o:        $(PHDRS) passwd.c
 pgsql.o:         $(PHDRS) pgsql.c
+readsock.o:      $(PHDRS) readsock.c
 redis.o:         $(PHDRS) redis.c
 spf.o:           $(PHDRS) spf.c
 sqlite.o:        $(PHDRS) sqlite.c
diff --git a/src/src/lookups/readsock.c b/src/src/lookups/readsock.c
new file mode 100644
index 0000000..c2088b7
--- /dev/null
+++ b/src/src/lookups/readsock.c
@@ -0,0 +1,319 @@
+/*************************************************
+*     Exim - an Internet mail transport agent    *
+*************************************************/
+
+/* Copyright (c) Jeremy Harris 2020 */
+/* See the file NOTICE for conditions of use and distribution. */
+
+#include "../exim.h"
+#include "lf_functions.h"
+
+
+static int
+internal_readsock_open(client_conn_ctx * cctx, const uschar * sspec,
+  int timeout, BOOL do_tls, uschar ** errmsg)
+{
+int sep = ',';
+uschar * ele;
+const uschar * server_name;
+host_item host;
+
+if (Ustrncmp(sspec, "inet:", 5) == 0)
+  {
+  int port;
+  uschar * port_name;
+
+  DEBUG(D_lookup)
+    debug_printf_indent("  new inet socket needed for readsocket\n");
+
+  server_name = sspec + 5;
+  port_name = Ustrrchr(server_name, ':');
+
+  /* Sort out the port */
+
+  if (!port_name)
+    {
+    /* expand_string_message results in an EXPAND_FAIL, from our
+    only caller.  Lack of it gets a SOCK_FAIL; we feed back via errmsg
+    for that, which gets copied to search_error_message. */
+
+    expand_string_message =
+      string_sprintf("missing port for readsocket %s", sspec);
+    return FAIL;
+    }
+  *port_name++ = 0;           /* Terminate server name */
+
+  if (isdigit(*port_name))
+    {
+    uschar *end;
+    port = Ustrtol(port_name, &end, 0);
+    if (end != port_name + Ustrlen(port_name))
+      {
+      expand_string_message =
+    string_sprintf("invalid port number %s", port_name);
+      return FAIL;
+      }
+    }
+  else
+    {
+    struct servent *service_info = getservbyname(CS port_name, "tcp");
+    if (!service_info)
+      {
+      expand_string_message = string_sprintf("unknown port \"%s\"",
+    port_name);
+      return FAIL;
+      }
+    port = ntohs(service_info->s_port);
+    }
+
+  /* Not having the request-string here in the open routine means
+  that we cannot do TFO; a pity */
+
+  cctx->sock = ip_connectedsocket(SOCK_STREAM, server_name, port, port,
+      timeout, &host, errmsg, NULL);
+  callout_address = NULL;
+  if (cctx->sock < 0)
+    return FAIL;
+  }
+
+else
+  {
+  struct sockaddr_un sockun;         /* don't call this "sun" ! */
+  int rc;
+
+  DEBUG(D_lookup)
+    debug_printf_indent("  new unix socket needed for readsocket\n");
+
+  if ((cctx->sock = socket(PF_UNIX, SOCK_STREAM, 0)) == -1)
+    {
+    *errmsg = string_sprintf("failed to create socket: %s", strerror(errno));
+    return FAIL;
+    }
+
+  sockun.sun_family = AF_UNIX;
+  sprintf(sockun.sun_path, "%.*s", (int)(sizeof(sockun.sun_path)-1),
+    sspec);
+  server_name = US sockun.sun_path;
+
+  sigalrm_seen = FALSE;
+  ALARM(timeout);
+  rc = connect(cctx->sock, (struct sockaddr *)(&sockun), sizeof(sockun));
+  ALARM_CLR(0);
+  if (sigalrm_seen)
+    {
+    *errmsg = US "socket connect timed out";
+    goto bad;
+    }
+  if (rc < 0)
+    {
+    *errmsg = string_sprintf("failed to connect to socket "
+      "%s: %s", sspec, strerror(errno));
+    goto bad;
+    }
+  host.name = server_name;
+  host.address = US"";
+  }
+
+#ifndef DISABLE_TLS
+if (do_tls)
+  {
+  smtp_connect_args conn_args = {.host = &host };
+  tls_support tls_dummy = {.sni=NULL};
+  uschar * errstr;
+
+  if (!tls_client_start(cctx, &conn_args, NULL, &tls_dummy, &errstr))
+    {
+    *errmsg = string_sprintf("TLS connect failed: %s", errstr);
+    goto bad;
+    }
+  }
+#endif
+
+DEBUG(D_expand|D_lookup) debug_printf_indent("  connected to socket %s\n", sspec);
+return OK;
+
+bad:
+  close(cctx->sock);
+  return FAIL;
+}
+
+/* All use of allocations will be done against the POOL_SEARCH memory,
+which is freed once by search_tidyup(). */
+
+/*************************************************
+*              Open entry point                  *
+*************************************************/
+
+/* See local README for interface description */
+/* We just create a placeholder record with a closed socket, so
+that connection cacheing at the framework layer works. */
+
+static void *
+readsock_open(const uschar * filename, uschar ** errmsg)
+{
+client_conn_ctx * cctx = store_get(sizeof(*cctx), FALSE);
+cctx->sock = -1;
+cctx->tls_ctx = NULL;
+DEBUG(D_lookup) debug_printf_indent("readsock: allocated context\n");
+return cctx;
+}
+
+
+
+
+
+/*************************************************
+*         Find entry point for lsearch           *
+*************************************************/
+
+/* See local README for interface description */
+
+static int
+readsock_find(void * handle, const uschar * filename, const uschar * keystring,
+  int length, uschar ** result, uschar ** errmsg, uint * do_cache,
+  const uschar * opts)
+{
+client_conn_ctx * cctx = handle;
+int sep = ',';
+struct {
+    BOOL do_shutdown:1;
+    BOOL do_tls:1;
+    BOOL cache:1;
+} lf = {.do_shutdown = TRUE};
+uschar * eol = NULL;
+int timeout = 5;
+FILE * fp;
+gstring * yield;
+int ret = DEFER;
+
+DEBUG(D_lookup) debug_printf_indent("readsock: file=\"%s\" key=\"%s\" len=%d opts=\"%s\"\n", filename, keystring, length, opts);
+
+/* Parse options */
+
+if (opts) for (uschar * s; s = string_nextinlist(&opts, &sep, NULL, 0); )
+  if (Ustrncmp(s, "timeout=", 8) == 0)
+    timeout = readconf_readtime(s + 8, 0, FALSE);
+  else if (Ustrncmp(s, "shutdown=", 9) == 0)
+    lf.do_shutdown = Ustrcmp(s + 9, "no") != 0;
+#ifndef DISABLE_TLS
+  else if (Ustrncmp(s, "tls=", 4) == 0 && Ustrcmp(s + 4, US"no") != 0)
+    lf.do_tls = TRUE;
+#endif
+  else if (Ustrncmp(s, "eol=", 4) == 0)
+    eol = s + 4;
+  else if (Ustrcmp(s, "cache=yes") == 0)
+    lf.cache = TRUE;
+  else if (Ustrcmp(s, "send=no") == 0)
+    length = 0;
+
+if (!filename) return FAIL;    /* Server spec is required */
+
+/* Open the socket, if not cached */
+
+if (cctx->sock == -1)
+  if (internal_readsock_open(cctx, filename, timeout, lf.do_tls, errmsg) != OK)
+    return ret;
+
+testharness_pause_ms(100);    /* Allow sequencing of test actions */
+
+/* Write the request string, if not empty or already done */
+
+if (length)
+  {
+  if ((
+#ifndef DISABLE_TLS
+      cctx->tls_ctx ? tls_write(cctx->tls_ctx, keystring, length, FALSE) :
+#endif
+              write(cctx->sock, keystring, length)) != length)
+    {
+    *errmsg = string_sprintf("request write to socket "
+      "failed: %s", strerror(errno));
+    goto out;
+    }
+  }
+
+/* Shut down the sending side of the socket. This helps some servers to
+recognise that it is their turn to do some work. Just in case some
+system doesn't have this function, make it conditional. */
+
+#ifdef SHUT_WR
+if (!cctx->tls_ctx && lf.do_shutdown)
+  shutdown(cctx->sock, SHUT_WR);
+#endif
+
+testharness_pause_ms(100);
+
+/* Now we need to read from the socket, under a timeout. The function
+that reads a file can be used.  If we're using a stdio buffered read,
+and might need later write ops on the socket, the stdio must be in
+writable mode or the underlying socket goes non-writable. */
+
+if (!cctx->tls_ctx)
+  fp = fdopen(cctx->sock, lf.do_shutdown ? "rb" : "wb");
+
+sigalrm_seen = FALSE;
+ALARM(timeout);
+yield =
+#ifndef DISABLE_TLS
+  cctx->tls_ctx ? cat_file_tls(cctx->tls_ctx, NULL, eol) :
+#endif
+          cat_file(fp, NULL, eol);
+ALARM_CLR(0);
+
+if (sigalrm_seen)
+  { *errmsg = US "socket read timed out"; goto out; }
+
+*result = yield ? string_from_gstring(yield) : US"";
+ret = OK;
+if (!lf.cache) *do_cache = 0;
+
+out:
+
+(void) close(cctx->sock);
+cctx->sock = -1;
+return ret;
+}
+
+
+
+/*************************************************
+*              Close entry point                 *
+*************************************************/
+
+/* See local README for interface description */
+
+static void
+readsock_close(void * handle)
+{
+client_conn_ctx * cctx = handle;
+if (cctx->sock < 0) return;
+#ifndef DISABLE_TLS
+if (cctx->tls_ctx) tls_close(cctx->tls_ctx, TRUE);
+#endif
+close(cctx->sock);
+cctx->sock = -1;
+}
+
+
+
+static lookup_info readsock_lookup_info = {
+  .name = US"readsock",            /* lookup name */
+  .type = lookup_querystyle,
+  .open = readsock_open,        /* open function */
+  .check = NULL,
+  .find = readsock_find,        /* find function */
+  .close = readsock_close,
+  .tidy = NULL,
+  .quote = NULL,            /* no quoting function */
+  .version_report = NULL
+};
+
+
+#ifdef DYNLOOKUP
+#define readsock_lookup_module_info _lookup_module_info
+#endif
+
+static lookup_info *_lookup_list[] = { &readsock_lookup_info };
+lookup_module_info readsock_lookup_module_info = { LOOKUP_MODULE_INFO_MAGIC, _lookup_list, 1 };
+
+/* End of lookups/readsock.c */
diff --git a/src/src/search.c b/src/src/search.c
index d929322..d3511df 100644
--- a/src/src/search.c
+++ b/src/src/search.c
@@ -239,12 +239,10 @@ Returns:     nothing
 static void
 tidyup_subtree(tree_node *t)
 {
-search_cache *c = (search_cache *)(t->data.ptr);
-if (t->left != NULL) tidyup_subtree(t->left);
-if (t->right != NULL) tidyup_subtree(t->right);
-if (c != NULL &&
-    c->handle != NULL &&
-    lookup_list[c->search_type]->close != NULL)
+search_cache * c = (search_cache *)(t->data.ptr);
+if (t->left)  tidyup_subtree(t->left);
+if (t->right) tidyup_subtree(t->right);
+if (c && c->handle && lookup_list[c->search_type]->close)
   lookup_list[c->search_type]->close(c->handle);
 }


@@ -522,7 +520,8 @@ else
   DEBUG(D_lookup)
     {
     if (t)
-      debug_printf_indent("cached data found but either wrong opts or dated; ");
+      debug_printf_indent("cached data found but %s; ",
+    e->expiry && e->expiry <= time(NULL) ? "out-of-date" : "wrong opts");
     debug_printf_indent("%s lookup required for %s%s%s\n",
       filename ? US"file" : US"database",
       keystring,
@@ -545,13 +544,10 @@ else


   else if (do_cache)
     {
-    if (!t) /* No existing entry.  Create new one. */
+    if (!t)    /* No existing entry.  Create new one. */
       {
       int len = keylength + 1;
       e = store_get(sizeof(expiring_data) + sizeof(tree_node) + len, is_tainted(keystring));
-      e->expiry = do_cache == UINT_MAX ? 0 : time(NULL)+do_cache;
-      e->opts = opts;
-      e->data.ptr = data;
       t = (tree_node *)(e+1);
       memcpy(t->name, keystring, len);
       t->data.ptr = e;
@@ -560,7 +556,7 @@ else
       /* Else previous, out-of-date cache entry.  Update with the */
       /* new result and forget the old one */
     e->expiry = do_cache == UINT_MAX ? 0 : time(NULL)+do_cache;
-    e->opts = opts;
+    e->opts = opts ? string_copy(opts) : NULL;
     e->data.ptr = data;
     }


@@ -570,7 +566,7 @@ else
   else
     {
     DEBUG(D_lookup) debug_printf_indent("lookup forced cache cleanup\n");
-    c->item_cache = NULL;
+    c->item_cache = NULL;     /* forget all lookups on this connection */
     }
   }


diff --git a/test/confs/0373 b/test/confs/0373
index 10d64b7..2909d7d 100644
--- a/test/confs/0373
+++ b/test/confs/0373
@@ -10,6 +10,8 @@ domainlist local_domains = test.ex : *.test.ex
acl_smtp_connect = connect
trusted_users = CALLER

+log_selector = +millisec
+

# ----- ACL -----

diff --git a/test/log/0373 b/test/log/0373
index 7082f4d..33bdb38 100644
--- a/test/log/0373
+++ b/test/log/0373
@@ -1 +1 @@
-1999-03-02 09:44:33 H=[V4NET.0.0.0] U=CALLER temporarily rejected connection in "connect" ACL: failed to expand ACL string "${readsocket{TESTSUITE/test-socket}{QUERY-ACL\n}{2s}{*EOL*}}": socket read timed out
+2017-07-30 18:51:05.712 H=[V4NET.0.0.0] U=CALLER temporarily rejected connection in "connect" ACL: failed to expand ACL string "${readsocket{TESTSUITE/test-socket}{QUERY-ACL\n}{2s}{*EOL*}}": socket read timed out
diff --git a/test/rejectlog/0373 b/test/rejectlog/0373
index 7082f4d..33bdb38 100644
--- a/test/rejectlog/0373
+++ b/test/rejectlog/0373
@@ -1 +1 @@
-1999-03-02 09:44:33 H=[V4NET.0.0.0] U=CALLER temporarily rejected connection in "connect" ACL: failed to expand ACL string "${readsocket{TESTSUITE/test-socket}{QUERY-ACL\n}{2s}{*EOL*}}": socket read timed out
+2017-07-30 18:51:05.712 H=[V4NET.0.0.0] U=CALLER temporarily rejected connection in "connect" ACL: failed to expand ACL string "${readsocket{TESTSUITE/test-socket}{QUERY-ACL\n}{2s}{*EOL*}}": socket read timed out
diff --git a/test/scripts/0000-Basic/0373 b/test/scripts/0000-Basic/0373
index 0f63cee..5d8bbee 100644
--- a/test/scripts/0000-Basic/0373
+++ b/test/scripts/0000-Basic/0373
@@ -2,6 +2,7 @@
 need_ipv4
 #
 exim -be
+connfail cases (no server)
 1 >>${readsocket{DIR/test-socket}{QUERY-1\n}}<<
 2 ${if exists{DIR/test-socket}\
   {>>${readsocket{DIR/test-socket}{QUERY-1\n}}<<}\
@@ -38,6 +39,7 @@ QUERY-9
 ****
 millisleep 500
 exim -be
+unix-socket cases
 1 >>${readsocket{DIR/test-socket}{QUERY-1\n}}<<
 2 >>${readsocket{DIR/test-socket}{QUERY-2\n}}<<
 3 >>${readsocket{DIR/test-socket}{QUERY-3\n}{2s}{*EOL*}}<<
@@ -90,19 +92,44 @@ QUERY-10
 ****
 millisleep 500
 exim -be
-1 >>${readsocket{inet:thisloop:PORT_S}{QUERY-1\n}}<<
-2 >>${readsocket{inet:127.0.0.1:PORT_S}{QUERY-2\n}}<<
-3 >>${readsocket{inet:127.0.0.1:PORT_S}{QUERY-3\n}{2s}{*EOL*}}<<
-4 >>${readsocket{inet:127.0.0.1:PORT_S}{QUERY-4\n}{2s}{*EOL*}{sock error}}<<
-5 >>${readsocket{inet:127.0.0.1:PORT_S}{}}<<
-6 >>${readsocket{inet:127.0.0.1:PORT_S}{QUERY-6\n}}<<
-7 >>${readsocket{inet:127.0.0.1:PORT_S}{QUERY-7\n}{1s}{}{sock error}}<<
-8 >>${readsocket{inet:127.0.0.1:PORT_S}{QUERY-8\n}{1s}}<<
-9 >>${readsocket{inet:127.0.0.1:PORT_S}{QUERY-9\n}{1s}{}{sock error}}<<
-10 >>${readsocket{inet:badloop:PORT_S}{QUERY-10\n}}<<
-11 >>${readsocket{inet:thisloop:PORT_S}{QUERY-11\n}{2s:shutdown=no}}<<
+ipv4 cases
+1  ANSWER-1      >>${readsocket{inet:thisloop:PORT_S}{QUERY-1\n}}<<
+2  ANSWER-2      >>${readsocket{inet:127.0.0.1:PORT_S}{QUERY-2\n}}<<
+3  ANSWER-3*EOL* >>${readsocket{inet:127.0.0.1:PORT_S}{QUERY-3\n}{2s}{*EOL*}}<<
+4  ANSWER-4*EOL* >>${readsocket{inet:127.0.0.1:PORT_S}{QUERY-4\n}{2s}{*EOL*}{sock error}}<<
+5  ANSWER-5      >>${readsocket{inet:127.0.0.1:PORT_S}{}}<<
+6                >>${readsocket{inet:127.0.0.1:PORT_S}{QUERY-6\n}}<<
+7                >>${readsocket{inet:127.0.0.1:PORT_S}{QUERY-7\n}{1s}{}{sock error}}<<
+8 read timed out >>${readsocket{inet:127.0.0.1:PORT_S}{QUERY-8\n}{1s}}<<
+9  sock error    >>${readsocket{inet:127.0.0.1:PORT_S}{QUERY-9\n}{1s}{}{sock error}}<<
+10 ANSWER-10\\n     >>${readsocket{inet:badloop:PORT_S}{QUERY-10\n}}<<
+11 ANSWER-11     >>${readsocket{inet:thisloop:PORT_S}{QUERY-11\n}{2s:shutdown=no}}<<
 ****
 #
 exim -be
 crash-regression-check >>${readsocket{inet:127.0.0.1:PORT_N}{}{}}<<
 ****
+#
+# Caching of response value
+server DIR/test-socket 3
+QUERY-1
+>LF>ANSWER-1
+>*eof
+QUERY-2
+>LF>ANSWER-2
+>*eof
+QUERY-1
+>LF>ANSWER-1
+>*eof
+****
+millisleep 500
+exim -be
+caching of response value
+1  >>${readsocket{DIR/test-socket}{QUERY-1\n}{5s:cache=yes}}<<
+1+ >>${readsocket{DIR/test-socket}{QUERY-1\n}{5s:cache=yes}}<<
+2  >>${readsocket{DIR/test-socket}{QUERY-2\n}{5s:cache=yes}}<<
+2- >>${readsocket{DIR/test-socket2}{QUERY-2\n}{5s:cache=yes}{}{expected failure}}<<
+1- >>${readsocket{DIR/test-socket2}{QUERY-1\n}{5s:cache=yes}{}{expected failure}}<<
+1+ >>${readsocket{DIR/test-socket}{QUERY-1\n}{5s:cache=yes}}<<
+1- >>${readsocket{DIR/test-socket}{QUERY-1\n}{5s}}<<
+****
diff --git a/test/stderr/2200 b/test/stderr/2200
index c832032..ba50dff 100644
--- a/test/stderr/2200
+++ b/test/stderr/2200
@@ -43,7 +43,7 @@ search_tidyup called
   LRU list:
   internal_search_find: file="NULL"
     type=dnsdb key="a=shorthost.test.ex" opts=NULL
-  cached data found but either wrong opts or dated;   database lookup required for a=shorthost.test.ex
+  cached data found but out-of-date;   database lookup required for a=shorthost.test.ex
   dnsdb key: shorthost.test.ex
   lookup yielded: 127.0.0.1
 LOG: MAIN
diff --git a/test/stderr/2201 b/test/stderr/2201
index fb618fc..8f9d0b5 100644
--- a/test/stderr/2201
+++ b/test/stderr/2201
@@ -176,7 +176,7 @@ search_find: file="NULL"
 LRU list:
 internal_search_find: file="NULL"
   type=dnsdb key="a=shorthost.test.ex" opts=NULL
-cached data found but either wrong opts or dated; database lookup required for a=shorthost.test.ex
+cached data found but out-of-date; database lookup required for a=shorthost.test.ex
 dnsdb key: shorthost.test.ex
 lookup yielded: 127.0.0.1
 LOG: MAIN
diff --git a/test/stderr/2610 b/test/stderr/2610
index 951ddec..b8eda85 100644
--- a/test/stderr/2610
+++ b/test/stderr/2610
@@ -264,7 +264,7 @@ check set acl_m0 = ok:   ${lookup mysql                    {select name from the
  LRU list:
  internal_search_find: file="NULL"
    type=mysql key="select name from them where id = 'c'" opts="servers=127.0.0.1::1223/test/root/"
- cached data found but either wrong opts or dated;  database lookup required for select name from them where id = 'c'
+ cached data found but wrong opts;  database lookup required for select name from them where id = 'c'
  MySQL query: "select name from them where id = 'c'" opts 'servers=127.0.0.1::1223/test/root/'
  MYSQL using cached connection for 127.0.0.1:1223/test/root
  MYSQL: no data found
@@ -278,7 +278,7 @@ check set acl_m0 = ok:   ${lookup mysql,servers=127.0.0.1::1223/test/root/
  LRU list:
  internal_search_find: file="NULL"
    type=mysql key="select name from them where id = 'c'" opts="servers=127.0.0.1::1223"
- cached data found but either wrong opts or dated;  database lookup required for select name from them where id = 'c'
+ cached data found but wrong opts;  database lookup required for select name from them where id = 'c'
  MySQL query: "select name from them where id = 'c'" opts 'servers=127.0.0.1::1223'
  MYSQL using cached connection for 127.0.0.1:1223/test/root
  MYSQL: no data found
diff --git a/test/stderr/2620 b/test/stderr/2620
index 337d518..2c37828 100644
--- a/test/stderr/2620
+++ b/test/stderr/2620
@@ -254,7 +254,7 @@ check set acl_m0 = ok:   ${lookup pgsql                    {select name from the
  LRU list:
  internal_search_find: file="NULL"
    type=pgsql key="select name from them where id = 'c'" opts="servers=SSPEC"
- cached data found but either wrong opts or dated;  database lookup required for select name from them where id = 'c'
+ cached data found but wrong opts;  database lookup required for select name from them where id = 'c'
  PostgreSQL query: "select name from them where id = 'c'" opts 'servers=SSPEC'
  lookup deferred: PostgreSQL server "SSPEC" not found in pgsql_servers
 warn: condition test deferred in ACL "check_recipient"
@@ -361,7 +361,7 @@ check set acl_m0 = ok:   ${lookup pgsql                    {select name from the
  LRU list:
  internal_search_find: file="NULL"
    type=pgsql key="select name from them where id = 'c'" opts="servers=SSPEC"
- cached data found but either wrong opts or dated;  database lookup required for select name from them where id = 'c'
+ cached data found but wrong opts;  database lookup required for select name from them where id = 'c'
  PostgreSQL query: "select name from them where id = 'c'" opts 'servers=SSPEC'
  lookup deferred: PostgreSQL server "SSPEC" not found in pgsql_servers
 warn: condition test deferred in ACL "check_recipient"
diff --git a/test/stdout/0373 b/test/stdout/0373
index a4acc65..513f364 100644
--- a/test/stdout/0373
+++ b/test/stdout/0373
@@ -1,6 +1,8 @@
+> connfail cases (no server)

> Failed: failed to connect to socket TESTSUITE/test-socket: No such file or directory
> 2 ++ no socket ++
>

+> unix-socket cases
> 1 >>ANSWER-1

<<
> 2 >>ANSWER-2<<

@@ -13,22 +15,36 @@
> 9 >>sock error<<
>

 451 Temporary local problem - please try later
-> 1 >>ANSWER-1
+> ipv4 cases
+> 1  ANSWER-1      >>ANSWER-1
 <<
-> 2 >>ANSWER-2<<
-> 3 >>ANSWER-3*EOL*<<
-> 4 >>ANSWER-4*EOL*<<
-> 5 >>ANSWER-5<<
-> 6 >><<
-> 7 >><<
+> 2  ANSWER-2      >>ANSWER-2<<
+> 3  ANSWER-3*EOL* >>ANSWER-3*EOL*<<
+> 4  ANSWER-4*EOL* >>ANSWER-4*EOL*<<
+> 5  ANSWER-5      >>ANSWER-5<<
+> 6                >><<
+> 7                >><<

> Failed: socket read timed out

-> 9 >>sock error<<
-> 10 >>ANSWER-10
-<<
-> 11 >>ANSWER-11
+> 9  sock error    >>sock error<<
+> 10 ANSWER-10\n     >>ANSWER-10
 <<
+> 11 ANSWER-11     >><<

>

-> Failed: bad time value NULL
+> Failed: failed to connect to any address for 127.0.0.1: Connection refused
+>
+> caching of response value
+> 1 >>ANSWER-1
+<<
+> 1+ >>ANSWER-1
+<<
+> 2 >>ANSWER-2
+<<
+> 2- >>expected failure<<
+> 1- >>expected failure<<
+> 1+ >>ANSWER-1
+<<
+> 1- >>ANSWER-1
+<<
>


******** SERVER ********
@@ -129,3 +145,19 @@ Connection request from [ip4.ip4.ip4.ip4]
>LF>ANSWER-11
>*eof

End of script
+Listening on TESTSUITE/test-socket ...
+Connection request
+QUERY-1
+>LF>ANSWER-1
+>*eof
+Listening on TESTSUITE/test-socket ...
+Connection request
+QUERY-2
+>LF>ANSWER-2
+>*eof
+Listening on TESTSUITE/test-socket ...
+Connection request
+QUERY-1
+>LF>ANSWER-1
+>*eof
+End of script