This is an automated email from the ASF dual-hosted git repository.
lidavidm pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-adbc.git
The following commit(s) were added to refs/heads/main by this push:
new 8c833663b refactor(c/driver_manager): replace std::regex in profile
interpolation (#4780)
8c833663b is described below
commit 8c833663b7112fa926893fe65e25e5850ca1a3fa
Author: Kevin Ushey <[email protected]>
AuthorDate: Sun Sep 13 16:53:36 2026 -0700
refactor(c/driver_manager): replace std::regex in profile interpolation
(#4780)
Profile interpolation currently constructs a `std::regex` to expand `{{
env_var(NAME) }}` expressions. Replace it with a linear scanner that
preserves whitespace trimming, overlapping delimiters, literal malformed
braces, and existing error messages. This removes the regex construction
path implicated in #4638.
The interpolation helper follows the existing internal test-helper
export convention (`InternalAdbc` prefix and `ADBC_EXPORT`) so
shared-library tests link on Linux and Windows.
The CRAN GCC 16 sanitizer environment was not reproduced locally.
Closes #4638
---
.github/workflows/r-check.yml | 3 +
c/driver_manager/adbc_driver_manager.cc | 3 +-
c/driver_manager/adbc_driver_manager_internal.h | 6 +-
c/driver_manager/adbc_driver_manager_profiles.cc | 60 ++++++++++++-------
c/driver_manager/adbc_driver_manager_test.cc | 73 +++++++++++++++++++++++
go/adbc/drivermgr/adbc_driver_manager.cc | 3 +-
go/adbc/drivermgr/adbc_driver_manager_internal.h | 6 +-
go/adbc/drivermgr/adbc_driver_manager_profiles.cc | 60 ++++++++++++-------
8 files changed, 162 insertions(+), 52 deletions(-)
diff --git a/.github/workflows/r-check.yml b/.github/workflows/r-check.yml
index d98f739e2..6af752d99 100644
--- a/.github/workflows/r-check.yml
+++ b/.github/workflows/r-check.yml
@@ -85,6 +85,9 @@ jobs:
extra-packages: any::rcmdcheck, local::../adbcdrivermanager
needs: check
working-directory: r/${{ inputs.pkg }}
+ # These packages do not need Pandoc. Auto-detection ignores the local
+ # dependency override and tries to resolve adbcdrivermanager from
CRAN.
+ install-pandoc: false
- name: Start postgres test database
if: inputs.pkg == 'adbcpostgresql' && runner.os == 'Linux'
diff --git a/c/driver_manager/adbc_driver_manager.cc
b/c/driver_manager/adbc_driver_manager.cc
index 1a9fe7f36..5accc002b 100644
--- a/c/driver_manager/adbc_driver_manager.cc
+++ b/c/driver_manager/adbc_driver_manager.cc
@@ -63,7 +63,6 @@
#include <cstring>
#include <filesystem>
#include <functional>
-#include <regex>
#include <string>
#include <unordered_map>
#include <utility>
@@ -683,7 +682,7 @@ AdbcStatusCode InternalInitializeProfile(TempDatabase* args,
// use try_emplace so we only add the option if there isn't
// already an option with the same name
std::string processed;
- CHECK_STATUS(ProcessProfileValue(keys[i], values[i], processed, error));
+ CHECK_STATUS(InternalAdbcProcessProfileValue(keys[i], values[i],
processed, error));
args->options.try_emplace(keys[i], processed);
}
diff --git a/c/driver_manager/adbc_driver_manager_internal.h
b/c/driver_manager/adbc_driver_manager_internal.h
index 88b02e64c..6f0f6c832 100644
--- a/c/driver_manager/adbc_driver_manager_internal.h
+++ b/c/driver_manager/adbc_driver_manager_internal.h
@@ -189,8 +189,10 @@ AdbcStatusCode LoadDriverFromRegistry(HKEY root, const
std::wstring& driver_name
#endif
// Profile loading
-AdbcStatusCode ProcessProfileValue(std::string_view key, std::string_view
value,
- std::string& out, struct AdbcError* error);
+ADBC_EXPORT
+AdbcStatusCode InternalAdbcProcessProfileValue(std::string_view key,
+ std::string_view value,
std::string& out,
+ struct AdbcError* error);
// Initialization
/// Temporary state while the database is being configured.
diff --git a/c/driver_manager/adbc_driver_manager_profiles.cc
b/c/driver_manager/adbc_driver_manager_profiles.cc
index 2ad571676..b5597d0d8 100644
--- a/c/driver_manager/adbc_driver_manager_profiles.cc
+++ b/c/driver_manager/adbc_driver_manager_profiles.cc
@@ -23,7 +23,7 @@
#include "adbc_driver_manager_internal.h"
#include <filesystem>
-#include <regex>
+#include <locale>
#include <string>
#include <unordered_map>
#include <utility>
@@ -204,29 +204,47 @@ struct ProfileVisitor {
};
// Public implementations (non-static for use across translation units)
-AdbcStatusCode ProcessProfileValue(std::string_view key, std::string_view
value,
- std::string& out, struct AdbcError* error) {
+AdbcStatusCode InternalAdbcProcessProfileValue(std::string_view key,
+ std::string_view value,
std::string& out,
+ struct AdbcError* error) {
if (value.empty()) {
out = "";
return ADBC_STATUS_OK;
}
- static const std::regex pattern(R"(\{\{\s*([^{}]*?)\s*\}\})");
- auto end_of_last_match = value.begin();
- auto begin = std::regex_iterator(value.begin(), value.end(), pattern);
- auto end = decltype(begin){};
- std::match_results<std::string_view::iterator>::difference_type
pos_last_match = 0;
+ static const std::locale locale;
+ size_t end_of_last_match = 0;
+ size_t start_match = 0;
out.resize(0);
- for (auto itr = begin; itr != end; ++itr) {
- auto match = *itr;
- auto pos_match = match.position();
- auto diff = pos_match - pos_last_match;
- auto start_match = end_of_last_match;
- std::advance(start_match, diff);
- out.append(end_of_last_match, start_match);
-
- const auto content = match[1].str();
+ while ((start_match = value.find("{{", start_match)) !=
std::string_view::npos) {
+ const auto end_match = value.find_first_of("{}", start_match + 2);
+ if (end_match == std::string_view::npos) {
+ break;
+ }
+ if (value[end_match] == '{') {
+ // Braces cannot occur inside an interpolation. Allow overlapping opening
+ // delimiters so that "{{{ env_var(NAME) }}" still expands the inner
pair.
+ start_match = end_match - 1;
+ continue;
+ }
+ if (end_match + 1 == value.size() || value[end_match + 1] != '}') {
+ start_match = end_match + 1;
+ continue;
+ }
+ out.append(value.substr(end_of_last_match, start_match -
end_of_last_match));
+
+ auto content_start = start_match + 2;
+ auto content_end = end_match;
+ while (content_start < content_end && std::isspace(value[content_start],
locale)) {
+ ++content_start;
+ }
+ while (content_start < content_end && std::isspace(value[content_end - 1],
locale)) {
+ --content_end;
+ }
+
+ const auto content =
+ std::string(value.substr(content_start, content_end - content_start));
if (content.rfind("env_var(", 0) != 0) {
std::string message = "In profile: unsupported interpolation type in key
`" +
std::string(key) + "`: `" + content + "`";
@@ -270,13 +288,11 @@ AdbcStatusCode ProcessProfileValue(std::string_view key,
std::string_view value,
#endif
out.append(env_var_value);
- auto length_match = match.length();
- pos_last_match = pos_match + length_match;
- end_of_last_match = start_match;
- std::advance(end_of_last_match, length_match);
+ end_of_last_match = end_match + 2;
+ start_match = end_of_last_match;
}
- out.append(end_of_last_match, value.end());
+ out.append(value.substr(end_of_last_match));
return ADBC_STATUS_OK;
}
diff --git a/c/driver_manager/adbc_driver_manager_test.cc
b/c/driver_manager/adbc_driver_manager_test.cc
index cecfc4b10..caa0dddad 100644
--- a/c/driver_manager/adbc_driver_manager_test.cc
+++ b/c/driver_manager/adbc_driver_manager_test.cc
@@ -1759,6 +1759,79 @@ TEST_F(ConnectionProfiles, DuplicateQuotedKey) {
UnsetProfilePath();
}
+TEST_F(ConnectionProfiles, ProcessProfileValueDelimiters) {
+ const std::vector<std::pair<std::string, std::string>> cases = {
+ {"", ""},
+ {"literal text", "literal text"},
+ {"{{env_var(ADBC_PROFILE_PATH)}}", "profile-value"},
+ {"{{ \t\n\r\f\venv_var(ADBC_PROFILE_PATH)\v\f\r\n\t }}",
"profile-value"},
+ {"before{{env_var(ADBC_PROFILE_PATH)}}after",
"beforeprofile-valueafter"},
+ {"{{env_var(ADBC_PROFILE_PATH)}}{{env_var(ADBC_PROFILE_PATH)}}",
+ "profile-valueprofile-value"},
+ {"{env_var(ADBC_PROFILE_PATH)}", "{env_var(ADBC_PROFILE_PATH)}"},
+ {"{{env_var(ADBC_PROFILE_PATH)", "{{env_var(ADBC_PROFILE_PATH)"},
+ {"{{env_var(ADBC_PROFILE_PATH)}", "{{env_var(ADBC_PROFILE_PATH)}"},
+ {"{{env_var(ADBC_PROFILE_PATH)} }", "{{env_var(ADBC_PROFILE_PATH)} }"},
+ {"{{env_var(ADBC_{PROFILE_PATH)}}", "{{env_var(ADBC_{PROFILE_PATH)}}"},
+ {"{{env_var(ADBC_}PROFILE_PATH)}}", "{{env_var(ADBC_}PROFILE_PATH)}}"},
+ {"{{{env_var(ADBC_PROFILE_PATH)}}}", "{profile-value}"},
+ {"{{outer {{env_var(ADBC_PROFILE_PATH)}} }}", "{{outer profile-value
}}"},
+ {"{{broken} {{env_var(ADBC_PROFILE_PATH)}}", "{{broken} profile-value"},
+ {"}}{{env_var(ADBC_PROFILE_PATH)}}{{", "}}profile-value{{"},
+ {std::string("before\0", 7) + "{{env_var(ADBC_PROFILE_PATH)}}" +
+ std::string("\0after", 6),
+ std::string("before\0profile-value\0after", 26)},
+ {std::string(10000, '{') + "env_var(ADBC_PROFILE_PATH)}}",
+ std::string(9998, '{') + "profile-value"},
+ };
+ for (const auto& [input, expected] : cases) {
+ SCOPED_TRACE(input);
+ std::string out = "previous output";
+ SetProfilePath("profile-value");
+ const auto status = InternalAdbcProcessProfileValue("foo", input, out,
&error);
+ UnsetProfilePath();
+ ASSERT_THAT(status, IsOkStatus(&error));
+ EXPECT_EQ(out, expected);
+ }
+}
+
+TEST_F(ConnectionProfiles, ProcessProfileValueErrors) {
+ const std::vector<std::pair<std::string, std::string>> cases = {
+ {"{{}}", "unsupported interpolation type in key `foo`: ``"},
+ {"{{ \t\n\r\f\v }}", "unsupported interpolation type in key `foo`: ``"},
+ {"{{ unknown() }}", "unsupported interpolation type in key `foo`:
`unknown()`"},
+ {"{{ env_var (NAME) }}",
+ "unsupported interpolation type in key `foo`: `env_var (NAME)`"},
+ {"{{ env_var(NAME }}",
+ "malformed env_var() in key `foo`: missing closing parenthesis"},
+ {"{{ env_var( }}", "malformed env_var() in key `foo`: missing closing
parenthesis"},
+ {"{{ env_var() }}",
+ "malformed env_var() in key `foo`: missing environment variable name"},
+ };
+ for (const auto& [input, expected] : cases) {
+ SCOPED_TRACE(input);
+ std::string out;
+ ASSERT_THAT(
+ InternalAdbcProcessProfileValue("foo", "prefix" + input + "suffix",
out, &error),
+ IsStatus(ADBC_STATUS_INVALID_ARGUMENT, &error));
+ EXPECT_STREQ(error.message, ("[Driver Manager] In profile: " +
expected).c_str());
+ EXPECT_EQ(out, "prefix");
+ if (error.release) {
+ error.release(&error);
+ }
+ }
+}
+
+TEST_F(ConnectionProfiles, ProcessProfileValueDoesNotRecurse) {
+ std::string out;
+ SetProfilePath("{{ unsupported() }}");
+ const auto status = InternalAdbcProcessProfileValue(
+ "foo", "{{env_var(ADBC_PROFILE_PATH)}}", out, &error);
+ UnsetProfilePath();
+ ASSERT_THAT(status, IsOkStatus(&error));
+ EXPECT_EQ(out, "{{ unsupported() }}");
+}
+
TEST_F(ConnectionProfiles, UseEnvVar) {
auto filepath = temp_dir / "profile.toml";
toml::table profile = toml::parse(R"|(
diff --git a/go/adbc/drivermgr/adbc_driver_manager.cc
b/go/adbc/drivermgr/adbc_driver_manager.cc
index 1a9fe7f36..5accc002b 100644
--- a/go/adbc/drivermgr/adbc_driver_manager.cc
+++ b/go/adbc/drivermgr/adbc_driver_manager.cc
@@ -63,7 +63,6 @@
#include <cstring>
#include <filesystem>
#include <functional>
-#include <regex>
#include <string>
#include <unordered_map>
#include <utility>
@@ -683,7 +682,7 @@ AdbcStatusCode InternalInitializeProfile(TempDatabase* args,
// use try_emplace so we only add the option if there isn't
// already an option with the same name
std::string processed;
- CHECK_STATUS(ProcessProfileValue(keys[i], values[i], processed, error));
+ CHECK_STATUS(InternalAdbcProcessProfileValue(keys[i], values[i],
processed, error));
args->options.try_emplace(keys[i], processed);
}
diff --git a/go/adbc/drivermgr/adbc_driver_manager_internal.h
b/go/adbc/drivermgr/adbc_driver_manager_internal.h
index 88b02e64c..6f0f6c832 100644
--- a/go/adbc/drivermgr/adbc_driver_manager_internal.h
+++ b/go/adbc/drivermgr/adbc_driver_manager_internal.h
@@ -189,8 +189,10 @@ AdbcStatusCode LoadDriverFromRegistry(HKEY root, const
std::wstring& driver_name
#endif
// Profile loading
-AdbcStatusCode ProcessProfileValue(std::string_view key, std::string_view
value,
- std::string& out, struct AdbcError* error);
+ADBC_EXPORT
+AdbcStatusCode InternalAdbcProcessProfileValue(std::string_view key,
+ std::string_view value,
std::string& out,
+ struct AdbcError* error);
// Initialization
/// Temporary state while the database is being configured.
diff --git a/go/adbc/drivermgr/adbc_driver_manager_profiles.cc
b/go/adbc/drivermgr/adbc_driver_manager_profiles.cc
index 2ad571676..b5597d0d8 100644
--- a/go/adbc/drivermgr/adbc_driver_manager_profiles.cc
+++ b/go/adbc/drivermgr/adbc_driver_manager_profiles.cc
@@ -23,7 +23,7 @@
#include "adbc_driver_manager_internal.h"
#include <filesystem>
-#include <regex>
+#include <locale>
#include <string>
#include <unordered_map>
#include <utility>
@@ -204,29 +204,47 @@ struct ProfileVisitor {
};
// Public implementations (non-static for use across translation units)
-AdbcStatusCode ProcessProfileValue(std::string_view key, std::string_view
value,
- std::string& out, struct AdbcError* error) {
+AdbcStatusCode InternalAdbcProcessProfileValue(std::string_view key,
+ std::string_view value,
std::string& out,
+ struct AdbcError* error) {
if (value.empty()) {
out = "";
return ADBC_STATUS_OK;
}
- static const std::regex pattern(R"(\{\{\s*([^{}]*?)\s*\}\})");
- auto end_of_last_match = value.begin();
- auto begin = std::regex_iterator(value.begin(), value.end(), pattern);
- auto end = decltype(begin){};
- std::match_results<std::string_view::iterator>::difference_type
pos_last_match = 0;
+ static const std::locale locale;
+ size_t end_of_last_match = 0;
+ size_t start_match = 0;
out.resize(0);
- for (auto itr = begin; itr != end; ++itr) {
- auto match = *itr;
- auto pos_match = match.position();
- auto diff = pos_match - pos_last_match;
- auto start_match = end_of_last_match;
- std::advance(start_match, diff);
- out.append(end_of_last_match, start_match);
-
- const auto content = match[1].str();
+ while ((start_match = value.find("{{", start_match)) !=
std::string_view::npos) {
+ const auto end_match = value.find_first_of("{}", start_match + 2);
+ if (end_match == std::string_view::npos) {
+ break;
+ }
+ if (value[end_match] == '{') {
+ // Braces cannot occur inside an interpolation. Allow overlapping opening
+ // delimiters so that "{{{ env_var(NAME) }}" still expands the inner
pair.
+ start_match = end_match - 1;
+ continue;
+ }
+ if (end_match + 1 == value.size() || value[end_match + 1] != '}') {
+ start_match = end_match + 1;
+ continue;
+ }
+ out.append(value.substr(end_of_last_match, start_match -
end_of_last_match));
+
+ auto content_start = start_match + 2;
+ auto content_end = end_match;
+ while (content_start < content_end && std::isspace(value[content_start],
locale)) {
+ ++content_start;
+ }
+ while (content_start < content_end && std::isspace(value[content_end - 1],
locale)) {
+ --content_end;
+ }
+
+ const auto content =
+ std::string(value.substr(content_start, content_end - content_start));
if (content.rfind("env_var(", 0) != 0) {
std::string message = "In profile: unsupported interpolation type in key
`" +
std::string(key) + "`: `" + content + "`";
@@ -270,13 +288,11 @@ AdbcStatusCode ProcessProfileValue(std::string_view key,
std::string_view value,
#endif
out.append(env_var_value);
- auto length_match = match.length();
- pos_last_match = pos_match + length_match;
- end_of_last_match = start_match;
- std::advance(end_of_last_match, length_match);
+ end_of_last_match = end_match + 2;
+ start_match = end_of_last_match;
}
- out.append(end_of_last_match, value.end());
+ out.append(value.substr(end_of_last_match));
return ADBC_STATUS_OK;
}
