From: Tom Lane <tgl@sss.pgh.pa.us>
To: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
Cc: pgsql-hackers@lists.postgresql.org
Cc: steven.winfield@cantabcapital.com
Subject: Re: BUG #16059: Tab-completion of filenames in COPY commands removes required quotes
Date: Mon, 13 Jan 2020 20:38:17 -0500
Message-ID: <22096.1578965897@sss.pgh.pa.us> (raw)
In-Reply-To: <28497.1578947794@sss.pgh.pa.us>
References: <16059-8836946734c02b84@postgresql.org>
<30313.1572729260@sss.pgh.pa.us>
<1628.1572820800@sss.pgh.pa.us>
<6ccd23e4-ce58-c2b5-39a1-801a9937ae86@2ndquadrant.com>
<12699.1578058657@sss.pgh.pa.us>
<32307.1578290788@sss.pgh.pa.us>
<1ed54e8f-d84e-bfbe-ffad-890015e12735@2ndquadrant.com>
<28497.1578947794@sss.pgh.pa.us>
I wrote:
> Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
>> I have found a weird behavior with identifier quoting, which is not the
>> subject of this patch, but it appears to be affected by it.
> I'll think about how to improve this. Given that we're dequoting
> filenames explicitly anyway, maybe we don't need to tell readline that
> double-quote is a quoting character. Another idea is that maybe
> we ought to refactor things so that identifier dequoting and requoting
> is handled explicitly, similarly to the way filenames work now.
> That would make the patch a lot bigger though.
On reflection, it seems like the best bet for the moment is to
remove double-quote from the rl_completer_quote_characters string,
which should restore all behavior around double-quoted strings to
what it was before. (We have to keep single-quote in that string,
though, or quoted file names fail entirely.)
The only impact this has on filename completion is that we're no
longer bright enough to convert a double-quoted filename to
single-quoted for you, which would be a nice-to-have but it's
hardly core functionality.
At some point it'd be good to undo that and upgrade quoted-identifier
processing, but I don't really want to include such changes in this
patch. I'm about burned out on tab completion for right now.
regards, tom lane
Attachments:psql-filename-completion-fixes-6.patch (text/x-diff, 24.9K) downloaddiff --git a/config/programs.m4 b/config/programs.m4index 90ff944..68ab823 100644--- a/config/programs.m4+++ b/config/programs.m4@@ -209,17 +209,20 @@ fi-# PGAC_VAR_RL_COMPLETION_APPEND_CHARACTER-# ---------------------------------------+# PGAC_READLINE_VARIABLES+# -----------------------
# Readline versions < 2.1 don't have rl_completion_append_character
+# Libedit lacks rl_filename_quote_characters and rl_filename_quoting_function-AC_DEFUN([PGAC_VAR_RL_COMPLETION_APPEND_CHARACTER],+AC_DEFUN([PGAC_READLINE_VARIABLES],
[AC_CACHE_CHECK([for rl_completion_append_character], pgac_cv_var_rl_completion_append_character,
[AC_LINK_IFELSE([AC_LANG_PROGRAM([#include <stdio.h>
-#ifdef HAVE_READLINE_READLINE_H-# include <readline/readline.h>+#if defined(HAVE_READLINE_READLINE_H)+#include <readline/readline.h>+#elif defined(HAVE_EDITLINE_READLINE_H)+#include <editline/readline.h>
#elif defined(HAVE_READLINE_H)
-# include <readline.h>+#include <readline.h>
#endif
],
[rl_completion_append_character = 'x';])],
@@ -228,7 +231,42 @@ AC_DEFUN([PGAC_VAR_RL_COMPLETION_APPEND_CHARACTER],
if test x"$pgac_cv_var_rl_completion_append_character" = x"yes"; then
AC_DEFINE(HAVE_RL_COMPLETION_APPEND_CHARACTER, 1,
[Define to 1 if you have the global variable 'rl_completion_append_character'.])
-fi])# PGAC_VAR_RL_COMPLETION_APPEND_CHARACTER+fi+AC_CACHE_CHECK([for rl_filename_quote_characters], pgac_cv_var_rl_filename_quote_characters,+[AC_LINK_IFELSE([AC_LANG_PROGRAM([#include <stdio.h>+#if defined(HAVE_READLINE_READLINE_H)+#include <readline/readline.h>+#elif defined(HAVE_EDITLINE_READLINE_H)+#include <editline/readline.h>+#elif defined(HAVE_READLINE_H)+#include <readline.h>+#endif+],+[rl_filename_quote_characters = "x";])],+[pgac_cv_var_rl_filename_quote_characters=yes],+[pgac_cv_var_rl_filename_quote_characters=no])])+if test x"$pgac_cv_var_rl_filename_quote_characters" = x"yes"; then+AC_DEFINE(HAVE_RL_FILENAME_QUOTE_CHARACTERS, 1,+ [Define to 1 if you have the global variable 'rl_filename_quote_characters'.])+fi+AC_CACHE_CHECK([for rl_filename_quoting_function], pgac_cv_var_rl_filename_quoting_function,+[AC_LINK_IFELSE([AC_LANG_PROGRAM([#include <stdio.h>+#if defined(HAVE_READLINE_READLINE_H)+#include <readline/readline.h>+#elif defined(HAVE_EDITLINE_READLINE_H)+#include <editline/readline.h>+#elif defined(HAVE_READLINE_H)+#include <readline.h>+#endif+],+[rl_filename_quoting_function = 0;])],+[pgac_cv_var_rl_filename_quoting_function=yes],+[pgac_cv_var_rl_filename_quoting_function=no])])+if test x"$pgac_cv_var_rl_filename_quoting_function" = x"yes"; then+AC_DEFINE(HAVE_RL_FILENAME_QUOTING_FUNCTION, 1,+ [Define to 1 if you have the global variable 'rl_filename_quoting_function'.])+fi+])# PGAC_READLINE_VARIABLESdiff --git a/configure b/configureindex 25cfbcb..a46ba40 100755--- a/configure+++ b/configure@@ -16316,10 +16316,12 @@ else
cat confdefs.h - <<_ACEOF >conftest.$ac_ext
/* end confdefs.h. */
#include <stdio.h>
-#ifdef HAVE_READLINE_READLINE_H-# include <readline/readline.h>+#if defined(HAVE_READLINE_READLINE_H)+#include <readline/readline.h>+#elif defined(HAVE_EDITLINE_READLINE_H)+#include <editline/readline.h>
#elif defined(HAVE_READLINE_H)
-# include <readline.h>+#include <readline.h>
#endif
int
@@ -16345,6 +16347,85 @@ if test x"$pgac_cv_var_rl_completion_append_character" = x"yes"; then
$as_echo "#define HAVE_RL_COMPLETION_APPEND_CHARACTER 1" >>confdefs.h
fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for rl_filename_quote_characters" >&5+$as_echo_n "checking for rl_filename_quote_characters... " >&6; }+if ${pgac_cv_var_rl_filename_quote_characters+:} false; then :+ $as_echo_n "(cached) " >&6+else+ cat confdefs.h - <<_ACEOF >conftest.$ac_ext+/* end confdefs.h. */+#include <stdio.h>+#if defined(HAVE_READLINE_READLINE_H)+#include <readline/readline.h>+#elif defined(HAVE_EDITLINE_READLINE_H)+#include <editline/readline.h>+#elif defined(HAVE_READLINE_H)+#include <readline.h>+#endif++int+main ()+{+rl_filename_quote_characters = "x";+ ;+ return 0;+}+_ACEOF+if ac_fn_c_try_link "$LINENO"; then :+ pgac_cv_var_rl_filename_quote_characters=yes+else+ pgac_cv_var_rl_filename_quote_characters=no+fi+rm -f core conftest.err conftest.$ac_objext \+ conftest$ac_exeext conftest.$ac_ext+fi+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv_var_rl_filename_quote_characters" >&5+$as_echo "$pgac_cv_var_rl_filename_quote_characters" >&6; }+if test x"$pgac_cv_var_rl_filename_quote_characters" = x"yes"; then++$as_echo "#define HAVE_RL_FILENAME_QUOTE_CHARACTERS 1" >>confdefs.h++fi+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for rl_filename_quoting_function" >&5+$as_echo_n "checking for rl_filename_quoting_function... " >&6; }+if ${pgac_cv_var_rl_filename_quoting_function+:} false; then :+ $as_echo_n "(cached) " >&6+else+ cat confdefs.h - <<_ACEOF >conftest.$ac_ext+/* end confdefs.h. */+#include <stdio.h>+#if defined(HAVE_READLINE_READLINE_H)+#include <readline/readline.h>+#elif defined(HAVE_EDITLINE_READLINE_H)+#include <editline/readline.h>+#elif defined(HAVE_READLINE_H)+#include <readline.h>+#endif++int+main ()+{+rl_filename_quoting_function = 0;+ ;+ return 0;+}+_ACEOF+if ac_fn_c_try_link "$LINENO"; then :+ pgac_cv_var_rl_filename_quoting_function=yes+else+ pgac_cv_var_rl_filename_quoting_function=no+fi+rm -f core conftest.err conftest.$ac_objext \+ conftest$ac_exeext conftest.$ac_ext+fi+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv_var_rl_filename_quoting_function" >&5+$as_echo "$pgac_cv_var_rl_filename_quoting_function" >&6; }+if test x"$pgac_cv_var_rl_filename_quoting_function" = x"yes"; then++$as_echo "#define HAVE_RL_FILENAME_QUOTING_FUNCTION 1" >>confdefs.h++fi+
for ac_func in rl_completion_matches rl_filename_completion_function rl_reset_screen_size
do :
as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh`
diff --git a/configure.in b/configure.inindex 1599fc5..8165f70 100644--- a/configure.in+++ b/configure.in@@ -1874,7 +1874,7 @@ fi
LIBS="$LIBS_including_readline"
if test "$with_readline" = yes; then
- PGAC_VAR_RL_COMPLETION_APPEND_CHARACTER+ PGAC_READLINE_VARIABLES
AC_CHECK_FUNCS([rl_completion_matches rl_filename_completion_function rl_reset_screen_size])
AC_CHECK_FUNCS([append_history history_truncate_file])
fi
diff --git a/src/bin/psql/stringutils.c b/src/bin/psql/stringutils.cindex c74e58d..c521749 100644--- a/src/bin/psql/stringutils.c+++ b/src/bin/psql/stringutils.c@@ -282,6 +282,7 @@ strip_quotes(char *source, char quote, char escape, int encoding)
* entails_quote - any of these present? need outer quotes
* quote - doubled within string, affixed to both ends
* escape - doubled within string
+ * force_quote - if true, quote the output even if it doesn't "need" it
* encoding - the active character-set encoding
*
* Do not use this as a substitute for PQescapeStringConn(). Use it for
@@ -289,12 +290,13 @@ strip_quotes(char *source, char quote, char escape, int encoding)
*/
char *
quote_if_needed(const char *source, const char *entails_quote,
- char quote, char escape, int encoding)+ char quote, char escape, bool force_quote,+ int encoding)
{
const char *src;
char *ret;
char *dst;
- bool need_quotes = false;+ bool need_quotes = force_quote;
Assert(source != NULL);
Assert(quote != '\0');
diff --git a/src/bin/psql/stringutils.h b/src/bin/psql/stringutils.hindex 1fb1cfc..4be172e 100644--- a/src/bin/psql/stringutils.h+++ b/src/bin/psql/stringutils.h@@ -22,6 +22,7 @@ extern char *strtokx(const char *s,
extern void strip_quotes(char *source, char quote, char escape, int encoding);
extern char *quote_if_needed(const char *source, const char *entails_quote,
- char quote, char escape, int encoding);+ char quote, char escape, bool force_quote,+ int encoding);
#endif /* STRINGUTILS_H */
diff --git a/src/bin/psql/t/010_tab_completion.pl b/src/bin/psql/t/010_tab_completion.plindex b35c4f8..c27f216 100644--- a/src/bin/psql/t/010_tab_completion.pl+++ b/src/bin/psql/t/010_tab_completion.pl@@ -60,6 +60,30 @@ delete $ENV{TERM};
# Some versions of readline inspect LS_COLORS, so for luck unset that too.
delete $ENV{LS_COLORS};
+# In a VPATH build, we'll be started in the source directory, but we want+# to run in the build directory so that we can use relative paths to+# access the tmp_check subdirectory; otherwise the output from filename+# completion tests is too variable.+if ($ENV{TESTDIR})+{+ chdir $ENV{TESTDIR} or die "could not chdir to \"$ENV{TESTDIR}\": $!";+}++# Create some junk files for filename completion testing.+my $FH;+open $FH, ">", "tmp_check/somefile"+ or die("could not create file \"tmp_check/somefile\": $!");+print $FH "some stuff\n";+close $FH;+open $FH, ">", "tmp_check/afile123"+ or die("could not create file \"tmp_check/afile123\": $!");+print $FH "more stuff\n";+close $FH;+open $FH, ">", "tmp_check/afile456"+ or die("could not create file \"tmp_check/afile456\": $!");+print $FH "other stuff\n";+close $FH;+
# fire up an interactive psql session
my $in = '';
my $out = '';
@@ -104,6 +128,15 @@ sub clear_query
return;
}
+# Clear current line to start over+# (this will work in an incomplete string literal, but it's less desirable+# than clear_query because we lose evidence in the history file)+sub clear_line+{+ check_completion("\025\n", qr/postgres=# /, "control-U works");+ return;+}+
# check basic command completion: SEL<tab> produces SELECT<space>
check_completion("SEL\t", qr/SELECT /, "complete SEL<tab> to SELECT");
@@ -142,6 +175,47 @@ check_completion("\\DRD\t", qr/drds /, "complete \\DRD<tab> to \\drds");
clear_query();
+# check filename completion+check_completion(+ "\\lo_import tmp_check/some\t",+ qr|tmp_check/somefile |,+ "filename completion with one possibility");++clear_query();++# note: readline might print a bell before the completion+check_completion(+ "\\lo_import tmp_check/af\t",+ qr|tmp_check/af\a?ile|,+ "filename completion with multiple possibilities");++clear_query();++# COPY requires quoting+# note: broken versions of libedit want to backslash the closing quote;+# not much we can do about that+check_completion(+ "COPY foo FROM tmp_check/some\t",+ qr|'tmp_check/somefile\\?' |,+ "quoted filename completion with one possibility");++clear_line();++check_completion(+ "COPY foo FROM tmp_check/af\t",+ qr|'tmp_check/afile|,+ "quoted filename completion with multiple possibilities");++# some versions of readline/libedit require two tabs here, some only need one+# also, some will offer the whole path name and some just the file name+# the quotes might appear, too+check_completion(+ "\t\t",+ qr|afile123'? +'?(tmp_check/)?afile456|,+ "offer multiple file choices");++clear_line();+
# send psql an explicit \q to shut it down, else pty won't close properly
$timer->start(5);
$in .= "\\q\n";
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.cindex 2fd8886..2871159 100644--- a/src/bin/psql/tab-complete.c+++ b/src/bin/psql/tab-complete.c@@ -41,6 +41,7 @@
#ifdef USE_READLINE
#include <ctype.h>
+#include <sys/stat.h>
#include "catalog/pg_am_d.h"
#include "catalog/pg_class_d.h"
@@ -63,6 +64,15 @@
#define rl_completion_matches completion_matches
#endif
+/*+ * Currently we assume that rl_filename_dequoting_function exists if+ * rl_filename_quoting_function does. If that proves not to be the case,+ * we'd need to test for the former, or possibly both, in configure.+ */+#ifdef HAVE_RL_FILENAME_QUOTING_FUNCTION+#define USE_FILENAME_QUOTING_FUNCTIONS 1+#endif+
/* word break characters */
#define WORD_BREAKS "\t\n@$><=;|&{() "
@@ -157,9 +167,11 @@ typedef struct SchemaQuery
static int completion_max_records;
/*
- * Communication variables set by COMPLETE_WITH_FOO macros and then used by- * the completion callback functions. Ugly but there is no better way.+ * Communication variables set by psql_completion (mostly in COMPLETE_WITH_FOO+ * macros) and then used by the completion callback functions. Ugly but there+ * is no better way.
*/
+static char completion_last_char; /* last char of input word */
static const char *completion_charp; /* to pass a string */
static const char *const *completion_charpp; /* to pass a list of strings */
static const char *completion_info_charp; /* to pass a second string */
@@ -167,6 +179,7 @@ static const char *completion_info_charp2; /* to pass a third string */
static const VersionedQuery *completion_vquery; /* to pass a VersionedQuery */
static const SchemaQuery *completion_squery; /* to pass a SchemaQuery */
static bool completion_case_sensitive; /* completion is case sensitive */
+static bool completion_force_quote; /* true to force-quote filenames */
/*
* A few macros to ease typing. You can use these to complete the given
@@ -1114,9 +1127,9 @@ static char **get_previous_words(int point, char **buffer, int *nwords);
static char *get_guctype(const char *varname);
-#ifdef NOT_USED-static char *quote_file_name(char *text, int match_type, char *quote_pointer);-static char *dequote_file_name(char *text, char quote_char);+#ifdef USE_FILENAME_QUOTING_FUNCTIONS+static char *quote_file_name(char *fname, int match_type, char *quote_pointer);+static char *dequote_file_name(char *fname, int quote_char);
#endif
@@ -1129,8 +1142,37 @@ initialize_readline(void)
rl_readline_name = (char *) pset.progname;
rl_attempted_completion_function = psql_completion;
+#ifdef USE_FILENAME_QUOTING_FUNCTIONS+ rl_filename_quoting_function = quote_file_name;+ rl_filename_dequoting_function = dequote_file_name;+#endif+
rl_basic_word_break_characters = WORD_BREAKS;
+ /*+ * We should include '"' in rl_completer_quote_characters too, but that+ * will require some upgrades to how we handle quoted identifiers, so+ * that's for another day.+ */+ rl_completer_quote_characters = "'";++ /*+ * Set rl_filename_quote_characters to "all possible characters",+ * otherwise Readline will skip filename quoting if it thinks a filename+ * doesn't need quoting. Readline actually interprets this as bytes, so+ * there are no encoding considerations here.+ */+#ifdef HAVE_RL_FILENAME_QUOTE_CHARACTERS+ {+ unsigned char *fqc = (unsigned char *) pg_malloc(256);++ for (int i = 0; i < 255; i++)+ fqc[i] = (unsigned char) (i + 1);+ fqc[255] = '\0';+ rl_filename_quote_characters = (const char *) fqc;+ }+#endif+
completion_max_records = 1000;
/*
@@ -1455,6 +1497,10 @@ psql_completion(const char *text, int start, int end)
char *text_copy = pnstrdup(rl_line_buffer + start, end - start);
text = text_copy;
+ /* Remember last char of the given input word. */+ completion_last_char = (end > start) ? text[end - start - 1] : '\0';++ /* We usually want the append character to be a space. */
#ifdef HAVE_RL_COMPLETION_APPEND_CHARACTER
rl_completion_append_character = ' ';
#endif
@@ -2265,13 +2311,19 @@ psql_completion(const char *text, int start, int end)
Matches("COPY", "BINARY", MatchAny))
COMPLETE_WITH("FROM", "TO");
/* If we have COPY [BINARY] <sth> FROM|TO, complete with filename */
- else if (Matches("COPY|\\copy", MatchAny, "FROM|TO") ||+ else if (Matches("COPY", MatchAny, "FROM|TO") ||
Matches("COPY", "BINARY", MatchAny, "FROM|TO"))
{
completion_charp = "";
+ completion_force_quote = true; /* COPY requires quoted filename */+ matches = rl_completion_matches(text, complete_from_files);+ }+ else if (Matches("\\copy", MatchAny, "FROM|TO"))+ {+ completion_charp = "";+ completion_force_quote = false;
matches = rl_completion_matches(text, complete_from_files);
}
-
/* Handle COPY [BINARY] <sth> FROM|TO filename */
else if (Matches("COPY|\\copy", MatchAny, "FROM|TO", MatchAny) ||
Matches("COPY", "BINARY", MatchAny, "FROM|TO", MatchAny))
@@ -3821,6 +3873,7 @@ psql_completion(const char *text, int start, int end)
"\\s|\\w|\\write|\\lo_import"))
{
completion_charp = "\\";
+ completion_force_quote = false;
matches = rl_completion_matches(text, complete_from_files);
}
@@ -4387,14 +4440,58 @@ complete_from_variables(const char *text, const char *prefix, const char *suffix
* This function wraps rl_filename_completion_function() to strip quotes from
* the input before searching for matches and to quote any matches for which
* the consuming command will require it.
+ *+ * Caller must set completion_charp to a zero- or one-character string+ * containing the escape character. This is necessary since \copy has no+ * escape character, but every other backslash command recognizes "\" as an+ * escape character.+ *+ * Caller must also set completion_force_quote to indicate whether to force+ * quotes around the result. (The SQL COPY command requires that.)
*/
static char *
complete_from_files(const char *text, int state)
{
+#ifdef USE_FILENAME_QUOTING_FUNCTIONS++ /*+ * If we're using a version of Readline that supports filename quoting+ * hooks, rely on those, and invoke rl_filename_completion_function()+ * without messing with its arguments. Readline does stuff internally+ * that does not work well at all if we try to handle dequoting here.+ * Instead, Readline will call quote_file_name() and dequote_file_name()+ * (see below) at appropriate times.+ *+ * ... or at least, mostly it will. There are some paths involving+ * unmatched file names in which Readline never calls quote_file_name(),+ * and if left to its own devices it will incorrectly append a quote+ * anyway. Set rl_completion_suppress_quote to prevent that. If we do+ * get to quote_file_name(), we'll clear this again. (Yes, this seems+ * like it's working around Readline bugs.)+ *+ * (For now, we assume that rl_completion_suppress_quote exists if the+ * filename quoting hooks do.)+ */+ rl_completion_suppress_quote = 1;++ /* If user typed a quote, force quoting (never remove user's quote) */+ if (*text == '\'')+ completion_force_quote = true;++ return rl_filename_completion_function(text, state);+#else++ /*+ * Otherwise, we have to do the best we can.+ */
static const char *unquoted_text;
char *unquoted_match;
char *ret = NULL;
+ /* If user typed a quote, force quoting (never remove user's quote) */+ if (*text == '\'')+ completion_force_quote = true;+
if (state == 0)
{
/* Initialization: stash the unquoted input. */
@@ -4411,22 +4508,40 @@ complete_from_files(const char *text, int state)
unquoted_match = rl_filename_completion_function(unquoted_text, state);
if (unquoted_match)
{
- /*- * Caller sets completion_charp to a zero- or one-character string- * containing the escape character. This is necessary since \copy has- * no escape character, but every other backslash command recognizes- * "\" as an escape character. Since we have only two callers, don't- * bother providing a macro to simplify this.- */+ struct stat statbuf;+ bool is_dir = (stat(unquoted_match, &statbuf) == 0 &&+ S_ISDIR(statbuf.st_mode) != 0);++ /* Re-quote the result, if needed. */
ret = quote_if_needed(unquoted_match, " \t\r\n\"`",
- '\'', *completion_charp, pset.encoding);+ '\'', *completion_charp,+ completion_force_quote,+ pset.encoding);
if (ret)
free(unquoted_match);
else
ret = unquoted_match;
++ /*+ * If it's a directory, replace trailing quote with a slash; this is+ * usually more convenient. (If we didn't quote, leave this to+ * libedit.)+ */+ if (*ret == '\'' && is_dir)+ {+ char *retend = ret + strlen(ret) - 1;++ Assert(*retend == '\'');+ *retend = '/';+ /* Try to prevent libedit from adding a space, too */+#ifdef HAVE_RL_COMPLETION_APPEND_CHARACTER+ rl_completion_append_character = '\0';+#endif+ }
}
return ret;
+#endif /* USE_FILENAME_QUOTING_FUNCTIONS */
}
@@ -4678,46 +4793,108 @@ get_guctype(const char *varname)
return guctype;
}
-#ifdef NOT_USED+#ifdef USE_FILENAME_QUOTING_FUNCTIONS
/*
- * Surround a string with single quotes. This works for both SQL and- * psql internal. Currently disabled because it is reported not to- * cooperate with certain versions of readline.+ * Quote a filename according to SQL rules, returning a malloc'd string.+ * completion_charp must point to escape character or '\0', and+ * completion_force_quote must be set correctly, as per comments for+ * complete_from_files().
*/
static char *
-quote_file_name(char *text, int match_type, char *quote_pointer)+quote_file_name(char *fname, int match_type, char *quote_pointer)
{
char *s;
- size_t length;+ struct stat statbuf;++ /* Quote if needed. */+ s = quote_if_needed(fname, " \t\r\n\"`",+ '\'', *completion_charp,+ completion_force_quote,+ pset.encoding);+ if (!s)+ s = pg_strdup(fname);++ /*+ * However, some of the time we have to strip the trailing quote from what+ * we send back. Never strip the trailing quote if the user already typed+ * one; otherwise, suppress the trailing quote if we have multiple/no+ * matches (because we don't want to add a quote if the input is seemingly+ * unfinished), or if the input was already quoted (because Readline will+ * do arguably-buggy things otherwise), or if the file does not exist, or+ * if it's a directory.+ */+ if (*s == '\'' &&+ completion_last_char != '\'' &&+ (match_type != SINGLE_MATCH ||+ (quote_pointer && *quote_pointer == '\'') ||+ stat(fname, &statbuf) != 0 ||+ S_ISDIR(statbuf.st_mode)))+ {+ char *send = s + strlen(s) - 1;++ Assert(*send == '\'');+ *send = '\0';+ }- (void) quote_pointer; /* not used */+ /*+ * And now we can let Readline do its thing with possibly adding a quote+ * on its own accord. (This covers some additional cases beyond those+ * dealt with above.)+ */+ rl_completion_suppress_quote = 0;++ /*+ * If user typed a leading quote character other than single quote (i.e.,+ * double quote), zap it, so that we replace it with the correct single+ * quote.+ */+ if (quote_pointer && *quote_pointer != '\'')+ *quote_pointer = '\0';- length = strlen(text) +(match_type == SINGLE_MATCH ? 3 : 2);- s = pg_malloc(length);- s[0] = '\'';- strcpy(s + 1, text);- if (match_type == SINGLE_MATCH)- s[length - 2] = '\'';- s[length - 1] = '\0';
return s;
}
+/*+ * Dequote a filename, if it's quoted.+ * completion_charp must point to escape character or '\0', as per+ * comments for complete_from_files().+ */
static char *
-dequote_file_name(char *text, char quote_char)+dequote_file_name(char *fname, int quote_char)
{
- char *s;- size_t length;+ char *unquoted_fname;++ /*+ * If quote_char is set, it's not included in "fname". We have to add it+ * or strtokx will not interpret the string correctly (notably, it won't+ * recognize escapes).+ */+ if (quote_char == '\'')+ {+ char *workspace = (char *) pg_malloc(strlen(fname) + 2);- if (!quote_char)- return pg_strdup(text);+ workspace[0] = quote_char;+ strcpy(workspace + 1, fname);+ unquoted_fname = strtokx(workspace, "", NULL, "'", *completion_charp,+ false, true, pset.encoding);+ free(workspace);+ }+ else+ unquoted_fname = strtokx(fname, "", NULL, "'", *completion_charp,+ false, true, pset.encoding);- length = strlen(text);- s = pg_malloc(length - 2 + 1);- strlcpy(s, text +1, length - 2 + 1);+ /* expect a NULL return for the empty string only */+ if (!unquoted_fname)+ {+ Assert(*fname == '\0');+ unquoted_fname = fname;+ }- return s;+ /* readline expects a malloc'd result that it is to free */+ return pg_strdup(unquoted_fname);
}
-#endif /* NOT_USED */++#endif /* USE_FILENAME_QUOTING_FUNCTIONS */
#endif /* USE_READLINE */
diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.inindex 050c48b..983d94e 100644--- a/src/include/pg_config.h.in+++ b/src/include/pg_config.h.in@@ -488,6 +488,14 @@
/* Define to 1 if you have the `rl_filename_completion_function' function. */
#undef HAVE_RL_FILENAME_COMPLETION_FUNCTION
+/* Define to 1 if you have the global variable 'rl_filename_quote_characters'.+ */+#undef HAVE_RL_FILENAME_QUOTE_CHARACTERS++/* Define to 1 if you have the global variable 'rl_filename_quoting_function'.+ */+#undef HAVE_RL_FILENAME_QUOTING_FUNCTION+
/* Define to 1 if you have the `rl_reset_screen_size' function. */
#undef HAVE_RL_RESET_SCREEN_SIZE
diff --git a/src/tools/msvc/Solution.pm b/src/tools/msvc/Solution.pmindex 909bded..979964c 100644--- a/src/tools/msvc/Solution.pm+++ b/src/tools/msvc/Solution.pm@@ -333,6 +333,8 @@ sub GenerateFiles
HAVE_RL_COMPLETION_APPEND_CHARACTER => undef,
HAVE_RL_COMPLETION_MATCHES => undef,
HAVE_RL_FILENAME_COMPLETION_FUNCTION => undef,
+ HAVE_RL_FILENAME_QUOTE_CHARACTERS => undef,+ HAVE_RL_FILENAME_QUOTING_FUNCTION => undef,
HAVE_RL_RESET_SCREEN_SIZE => undef,
HAVE_SECURITY_PAM_APPL_H => undef,
HAVE_SETPROCTITLE => undef,
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-bugs@postgresql.org
Cc: tgl@sss.pgh.pa.us, peter.eisentraut@2ndquadrant.com, pgsql-hackers@lists.postgresql.org, steven.winfield@cantabcapital.com
Subject: Re: BUG #16059: Tab-completion of filenames in COPY commands removes required quotes
In-Reply-To: <22096.1578965897@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox