Skip to content

Commit 962e681

Browse files
committed
mpgen: Avoid deprecated SchemaParser::parseDiskFile call
This avoids deprecation warnings, and is potentially a little safer since it sandboxes the parsing and makes it an error to import paths outside the source directory. This change does have some downsides, though. It complicates include logic, and also drops compatibility with capnproto versions before 0.7.0 that don't have the new SchemaParser::parseFromDirectory method which was added in capnproto/capnproto@c1fe2b0 It also adds a hard dependency on the kj/filesystem.h library which was previously optional. Fixes #39
1 parent e8e89df commit 962e681

File tree

2 files changed

+33
-21
lines changed

2 files changed

+33
-21
lines changed

cmake/capnp_compat.cmake

+6-10
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,8 @@
22
# Distributed under the MIT software license, see the accompanying
33
# file COPYING or http://www.opensource.org/licenses/mit-license.php.
44

5-
# CMake target definitions for backwards compatibility with Ubuntu bionic
6-
# capnproto 0.6.1 package (https://packages.ubuntu.com/bionic/libcapnp-dev)
7-
8-
include(CheckIncludeFileCXX)
9-
include(CMakePushCheckState)
5+
# Define capnp_PREFIX if not defined to avoid issue on macos
6+
# https://github.com/chaincodelabs/libmultiprocess/issues/26
107

118
if (NOT DEFINED capnp_PREFIX AND DEFINED CAPNP_INCLUDE_DIRS)
129
get_filename_component(capnp_PREFIX "${CAPNP_INCLUDE_DIRS}" DIRECTORY)
@@ -16,6 +13,10 @@ if (NOT DEFINED CAPNPC_OUTPUT_DIR)
1613
set(CAPNPC_OUTPUT_DIR "${CMAKE_CURRENT_BINARY_DIR}")
1714
endif()
1815

16+
# CMake target definitions for backwards compatibility with Ubuntu bionic
17+
# capnproto 0.6.1 package (https://packages.ubuntu.com/bionic/libcapnp-dev)
18+
# https://github.com/chaincodelabs/libmultiprocess/issues/27
19+
1920
if (NOT DEFINED CAPNP_LIB_CAPNPC AND DEFINED CAPNP_LIB_CAPNP-RPC)
2021
string(REPLACE "-rpc" "c" CAPNP_LIB_CAPNPC "${CAPNP_LIB_CAPNP-RPC}")
2122
endif()
@@ -53,8 +54,3 @@ if (NOT TARGET CapnProto::kj-async AND DEFINED CAPNP_LIB_KJ-ASYNC)
5354
add_library(CapnProto::kj-async SHARED IMPORTED)
5455
set_target_properties(CapnProto::kj-async PROPERTIES IMPORTED_LOCATION "${CAPNP_LIB_KJ-ASYNC}")
5556
endif()
56-
57-
cmake_push_check_state()
58-
set(CMAKE_REQUIRED_INCLUDES ${CMAKE_REQUIRED_INCLUDES} ${CAPNP_INCLUDE_DIRS})
59-
check_include_file_cxx("kj/filesystem.h" HAVE_KJ_FILESYSTEM)
60-
cmake_pop_check_state()

src/mp/gen.cpp

+27-11
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,9 @@ bool BoxedType(const ::capnp::Type& type)
126126
void Generate(kj::StringPtr src_prefix,
127127
kj::StringPtr include_prefix,
128128
kj::StringPtr src_file,
129-
kj::ArrayPtr<const kj::StringPtr> import_paths)
129+
const std::vector<kj::StringPtr>& import_paths,
130+
const kj::ReadableDirectory& src_dir,
131+
const std::vector<kj::Own<const kj::ReadableDirectory>>& import_dirs)
130132
{
131133
std::string output_path;
132134
if (src_prefix == ".") {
@@ -176,7 +178,11 @@ void Generate(kj::StringPtr src_prefix,
176178
}
177179

178180
capnp::SchemaParser parser;
179-
auto file_schema = parser.parseDiskFile(src_file, src_file, import_paths);
181+
auto directory_pointers = kj::heapArray<const kj::ReadableDirectory*>(import_dirs.size());
182+
for (size_t i = 0; i < import_dirs.size(); ++i) {
183+
directory_pointers[i] = import_dirs[i].get();
184+
}
185+
auto file_schema = parser.parseFromDirectory(src_dir, kj::Path::parse(output_path), directory_pointers);
180186

181187
std::ofstream cpp_server(output_path + ".proxy-server.c++");
182188
cpp_server << "// Generated by " PROXY_BIN " from " << src_file << "\n\n";
@@ -611,20 +617,30 @@ int main(int argc, char** argv)
611617
exit(1);
612618
}
613619
std::vector<kj::StringPtr> import_paths;
614-
#ifdef HAVE_KJ_FILESYSTEM
620+
std::vector<kj::Own<const kj::ReadableDirectory>> import_dirs;
615621
auto fs = kj::newDiskFilesystem();
616622
auto cwd = fs->getCurrentPath();
617-
#endif
623+
kj::Own<const kj::ReadableDirectory> src_dir;
624+
KJ_IF_MAYBE(dir, fs->getRoot().tryOpenSubdir(cwd.evalNative(argv[1]))) {
625+
src_dir = kj::mv(*dir);
626+
} else {
627+
throw std::runtime_error(std::string("Failed to open src_prefix prefix directory: ") + argv[1]);
628+
}
618629
for (size_t i = 4; i < argc; ++i) {
619-
import_paths.emplace_back(argv[i]);
630+
KJ_IF_MAYBE(dir, fs->getRoot().tryOpenSubdir(cwd.evalNative(argv[i]))) {
631+
import_paths.emplace_back(argv[i]);
632+
import_dirs.emplace_back(kj::mv(*dir));
633+
} else {
634+
throw std::runtime_error(std::string("Failed to open import directory: ") + argv[i]);
635+
}
620636
}
621637
for (const char* path : {CMAKE_INSTALL_PREFIX "/include", capnp_PREFIX "/include"}) {
622-
#ifdef HAVE_KJ_FILESYSTEM
623-
KJ_IF_MAYBE(dir, fs->getRoot().tryOpenSubdir(cwd.evalNative(path))) { import_paths.emplace_back(path); }
624-
#else
625-
import_paths.emplace_back(path);
626-
#endif
638+
KJ_IF_MAYBE(dir, fs->getRoot().tryOpenSubdir(cwd.evalNative(path))) {
639+
import_paths.emplace_back(path);
640+
import_dirs.emplace_back(kj::mv(*dir));
641+
}
642+
// No exception thrown if _PREFIX directories do not exist
627643
}
628-
Generate(argv[1], argv[2], argv[3], {import_paths.data(), import_paths.size()});
644+
Generate(argv[1], argv[2], argv[3], import_paths, *src_dir, import_dirs);
629645
return 0;
630646
}

0 commit comments

Comments
 (0)