From 2b8d544bceb97e592a0640d5519ca0e0b08e5bcd Mon Sep 17 00:00:00 2001 From: Jekyll Wu Date: Tue, 5 Jun 2012 04:45:07 +0800 Subject: [PATCH] Refacotor the code for parsing and expanding environment variables. The old cold is copied from kio/kurlcompletion.cpp, which is not very suitable for detecting and expanding environment variables. For example, it only sees ' ' and '/' as the end point of an environment variable. That assumption fails easily with the common example of "PATH=$PATH:~/bin" --- src/ShellCommand.cpp | 95 +++++++++++++++++++--------------- src/ShellCommand.h | 8 +++ src/tests/ShellCommandTest.cpp | 18 ++++++- src/tests/ShellCommandTest.h | 2 + 4 files changed, 78 insertions(+), 45 deletions(-) diff --git a/src/ShellCommand.cpp b/src/ShellCommand.cpp index 696e10ead..3270d3ad6 100644 --- a/src/ShellCommand.cpp +++ b/src/ShellCommand.cpp @@ -25,10 +25,6 @@ using Konsole::ShellCommand; -// expands environment variables in 'text' -// function copied from kdelibs/kio/kio/kurlcompletion.cpp -static bool expandEnv(QString& text); - ShellCommand::ShellCommand(const QString& aCommand) { _arguments = KShell::splitArgs(aCommand); @@ -92,56 +88,69 @@ QString ShellCommand::expand(const QString& text) return result; } +bool ShellCommand::isValidEnvCharacter(const QChar& ch) +{ + const ushort code = ch.unicode(); + return isValidLeadingEnvCharacter(ch) || ('0' <= code && code <= '9'); + +} + +bool ShellCommand::isValidLeadingEnvCharacter(const QChar& ch) +{ + const ushort code = ch.unicode(); + return (code == '_') || ('A' <= code && code <= 'Z'); +} + /* * expandEnv * * Expand environment variables in text. Escaped '$' characters are ignored. * Return true if any variables were expanded */ -static bool expandEnv(QString& text) +bool ShellCommand::expandEnv(QString& text) { - // Find all environment variables beginning with '$' - // - int pos = 0; + const QLatin1Char dollarChar('$'); + const QLatin1Char backslashChar('\\'); + int dollarPos = 0; bool expanded = false; - while ((pos = text.indexOf(QLatin1Char('$'), pos)) != -1) { - // Skip escaped '$' - // - if (pos > 0 && text.at(pos - 1) == QLatin1Char('\\')) { - pos++; - // Variable found => expand - // + // find and expand all environment variables beginning with '$' + while ((dollarPos = text.indexOf(dollarChar, dollarPos)) != -1) { + + // if '$' is the last character, there is no way of expanding + if (dollarPos == text.length() -1 ) { + break; + } + + // skip escaped '$' + if (dollarPos > 0 && text.at(dollarPos - 1) == backslashChar) { + dollarPos++; + continue; + } + + // if '$' is followed by an invalid leading character, skip this '$' + if (!isValidLeadingEnvCharacter(text.at(dollarPos + 1))) { + dollarPos++; + continue; + } + + int endPos = dollarPos + 1; + Q_ASSERT( endPos < text.length()); + while (endPos < text.length() && isValidEnvCharacter(text.at(endPos))) { + endPos++; + } + + const int len = endPos - dollarPos; + const QString key = text.mid(dollarPos + 1, len - 1); + const QString value = QString::fromLocal8Bit(qgetenv(key.toLocal8Bit())); + + if (!value.isEmpty()) { + text.replace(dollarPos, len, value); + expanded = true; + dollarPos = dollarPos + value.length(); } else { - // Find the end of the variable = next '/' or ' ' - // - int pos2 = text.indexOf(QLatin1Char(' '), pos + 1); - const int pos_tmp = text.indexOf(QLatin1Char('/'), pos + 1); - - if (pos2 == -1 || (pos_tmp != -1 && pos_tmp < pos2)) - pos2 = pos_tmp; - - if (pos2 == -1) - pos2 = text.length(); - - // Replace if the variable is terminated by '/' or ' ' - // and defined - // - if (pos2 >= 0) { - const int len = pos2 - pos; - const QString key = text.mid(pos + 1, len - 1); - const QString value = - QString::fromLocal8Bit(qgetenv(key.toLocal8Bit())); - - if (!value.isEmpty()) { - expanded = true; - text.replace(pos, len, value); - pos = pos + value.length(); - } else { - pos = pos2; - } - } + dollarPos = endPos; } } diff --git a/src/ShellCommand.h b/src/ShellCommand.h index 49331bdcd..5c80e914c 100644 --- a/src/ShellCommand.h +++ b/src/ShellCommand.h @@ -84,7 +84,15 @@ public: /** Expands environment variables in each string in @p list. */ static QStringList expand(const QStringList& items); + + static bool isValidEnvCharacter(const QChar& ch); + + static bool isValidLeadingEnvCharacter(const QChar& ch); + private: + + static bool expandEnv(QString& text); + QStringList _arguments; }; } diff --git a/src/tests/ShellCommandTest.cpp b/src/tests/ShellCommandTest.cpp index e41725719..8b48daedf 100644 --- a/src/tests/ShellCommandTest.cpp +++ b/src/tests/ShellCommandTest.cpp @@ -60,15 +60,29 @@ void ShellCommandTest::testConstructorWithTwoArguments() void ShellCommandTest::testExpandEnvironmentVariable() { - QString text = "DIR=$PATH"; + QString text = "PATH=$PATH:~/bin"; const QString env = "PATH"; const QString value = "/usr/sbin:/sbin:/usr/local/bin:/usr/bin:/bin"; qputenv(env.toLocal8Bit(), value.toLocal8Bit()); const QString result = ShellCommand::expand(text); - const QString expected = text.remove('$').replace(env, value); + const QString expected = text.replace("$" + env, value); QCOMPARE(result, expected); } +void ShellCommandTest::testValidEnvCharacter() +{ + QChar validChar('A'); + const bool result = ShellCommand::isValidEnvCharacter(validChar); + QCOMPARE(result, true); +} + +void ShellCommandTest::testValidLeadingEnvCharacter() +{ + QChar invalidChar('9'); + const bool result = ShellCommand::isValidLeadingEnvCharacter(invalidChar); + QCOMPARE(result, false); +} + QTEST_KDEMAIN_CORE(ShellCommandTest) #include "ShellCommandTest.moc" diff --git a/src/tests/ShellCommandTest.h b/src/tests/ShellCommandTest.h index b8c00c6cd..fcf85c1a0 100644 --- a/src/tests/ShellCommandTest.h +++ b/src/tests/ShellCommandTest.h @@ -36,6 +36,8 @@ private slots: void testConstructorWithOneArguemnt(); void testConstructorWithTwoArguments(); void testExpandEnvironmentVariable(); + void testValidEnvCharacter(); + void testValidLeadingEnvCharacter(); };