Skip to content

Commit

Permalink
Add support for single quote parsing
Browse files Browse the repository at this point in the history
This enables to use single quotes, for example:

  pull-filter ignore 'route 1.2.3.4'
  verify-x509-name 'vpn.openvpn.net' (quote is not a part of a name)

Single quotes are treated as regular characters when inside
double quotes and vise versa.

OVPN3-921

Signed-off-by: Lev Stipakov <lev@openvpn.net>
  • Loading branch information
lstipakov authored and Jenkins-dev committed Oct 29, 2024
1 parent 10f5755 commit d8d7409
Show file tree
Hide file tree
Showing 5 changed files with 113 additions and 26 deletions.
62 changes: 53 additions & 9 deletions openvpn/common/lex.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,59 @@ struct SpaceMatch
}
};

/**
* @brief Helper class to handle quote processing
*
* This class provides functionality to handle single and double quotes within a text.
* It allows treating single quotes as regular characters when inside double quotes,
* and vice versa.
*/
class LexQuoteMixin
{
public:
/**
* @brief Check if currently inside a quote
* @return true if inside single or double quote, false otherwise
*/
bool in_quote() const
{
return in_squote | in_dquote;
}

protected:
/**
* @brief Handle a character as a potential quote
*
* Toggles quote state if `c` is a single or double quote,
* considering the current context.
*
* @param c Character to process
* @return true if `c` is treated as a quote, false otherwise
*/
bool handle_quote(char c)
{
if ((c == '\"') && (!in_squote))
{
in_dquote = !in_dquote;
return true;
}

if ((c == '\'') && (!in_dquote))
{
in_squote = !in_squote;
return true;
}

return false;
}

private:
bool in_squote = false; /**< State for single quotes */
bool in_dquote = false; /**< State for double quotes */
};

// A basic lexical analyzer that understands quoting
class StandardLex
class StandardLex : public LexQuoteMixin
{
public:
void put(char c)
Expand All @@ -56,9 +107,8 @@ class StandardLex
backslash_ = true;
ch = -1;
}
else if (c == '\"')
else if (handle_quote(c))
{
in_quote_ = !in_quote_;
ch = -1;
}
else
Expand All @@ -77,18 +127,12 @@ class StandardLex
{
ch = -1;
}

bool in_quote() const
{
return in_quote_;
}
bool in_backslash() const
{
return in_backslash_;
}

private:
bool in_quote_ = false;
bool backslash_ = false;
bool in_backslash_ = false;
int ch = -1;
Expand Down
24 changes: 7 additions & 17 deletions openvpn/common/options.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -540,13 +540,10 @@ class OptionList : public std::vector<Option>, public RCCopyable<thread_unsafe_r
typedef StandardLex Lex;

// special lex filter that recognizes end-of-line comments
class LexComment
class LexComment : public LexQuoteMixin
{
public:
LexComment()
: in_quote_(false), in_comment(false), backslash(false), ch(-1)
{
}
LexComment() = default;

void put(char c)
{
Expand All @@ -564,12 +561,11 @@ class OptionList : public std::vector<Option>, public RCCopyable<thread_unsafe_r
backslash = true;
ch = -1;
}
else if (c == '\"')
else if (handle_quote(c))
{
in_quote_ = !in_quote_;
ch = -1;
}
else if (is_comment(c) && !in_quote_)
else if (is_comment(c) && !in_quote())
{
in_comment = true;
ch = -1;
Expand All @@ -593,16 +589,10 @@ class OptionList : public std::vector<Option>, public RCCopyable<thread_unsafe_r
ch = -1;
}

bool in_quote() const
{
return in_quote_;
}

private:
bool in_quote_;
bool in_comment;
bool backslash;
int ch;
bool in_comment = false;
bool backslash = false;
int ch = -1;
};

class Limits
Expand Down
31 changes: 31 additions & 0 deletions test/unittests/test_optfilt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,37 @@ TEST(PushedOptionsFilter, PullFilterMalformedLong)
ASSERT_THROW(PushedOptionsFilter x(cfg), option_error);
}

TEST(PushedOptionsFilter, PullFilterSingleQuote)
{
OptionList cfg;
cfg.parse_from_config("pull-filter ignore 'route 1.2.3.4'", nullptr);
cfg.update_map();
PushedOptionsFilter filter(cfg);

OptionList src;
OptionList dst;

dst.extend(src, &filter);

testLog->startCollecting();
src.parse_from_config("route 1.1.1.1\nroute 2.2.2.2\nroute 1.2.3.4", nullptr);
dst.extend(src, &filter);
std::string filter_output(testLog->stopCollecting());

ASSERT_EQ(2u, dst.size())
<< "Too many options have been accepted by --pull-filter" << std::endl
<< filter_output;
}

TEST(PushedOptionsFilter, PullFilterMisplacedQuote)
{
OptionList cfg;
cfg.parse_from_config("pull-filter ignore 'a b' c", nullptr);
cfg.update_map();

ASSERT_THROW(PushedOptionsFilter x(cfg), option_error);
}

TEST(PushedOptionsFilter, PullFilterIgnoreAll)
{
OptionList cfg;
Expand Down
14 changes: 14 additions & 0 deletions test/unittests/test_parseargv.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -234,3 +234,17 @@ TEST(argv, parsetest1)
options_csv_test("V4,dev-type tun,link-mtu 1558,tun-mtu 1500,proto UDPv4,comp-lzo,keydir 1,cipher AES-256-CBC,auth SHA1,keysize 256,tls-auth,key-method 2,tls-client",
"");
}

TEST(argv, QuotesTest)
{
auto str = "\"abc,def\",\'abc,def\',\"abc',def\",\'abc\"def\',\'abc\"";
std::vector<std::string> list = Split::by_char<std::vector<std::string>, StandardLex, Split::NullLimit>(str, ',');
std::stringstream s;
std::copy(list.begin(), list.end(), std::ostream_iterator<std::string>(s, "\n"));
ASSERT_EQ("\"abc,def\"\n"
"\'abc,def'\n"
"\"abc',def\"\n"
"\'abc\"def\'\n"
"\'abc\"\n",
s.str());
}
8 changes: 8 additions & 0 deletions test/unittests/test_verify_x509_name.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,14 @@ TEST(VerifyX509Name, config_correct_name)
VerifyX509Name ok_name(parse_testcfg(config));
}

TEST(VerifyX509Name, config_squote)
{
// ensure that single quote is not treated as name part
std::string config = "verify-x509-name 'server.example.org'";
VerifyX509Name verify(parse_testcfg(config));
ASSERT_TRUE(verify.verify("server.example.org"));
}

TEST(VerifyX509Name, config_correct_name_prefix)
{
// Correct - type: name-prefix
Expand Down

0 comments on commit d8d7409

Please sign in to comment.