From aaf5598a032314767721fead8a0acf9ca37c5e09 Mon Sep 17 00:00:00 2001 From: Wouter van Oortmerssen Date: Thu, 18 May 2017 17:18:43 -0700 Subject: [PATCH] Standardized internal path handling on Posix separators. There were several possible bugs involving paths not being recognized as being the same on Windows. Rather than trying to ensure all code deals with / and \ correctly, paths now get transformed to / on input, fixing all current and future such bugs. Tested: on OS X. --- include/flatbuffers/idl.h | 2 ++ include/flatbuffers/util.h | 37 ++++++++++++++++++++++++++----------- src/flatc.cpp | 25 +++++++++++++++++-------- src/idl_parser.cpp | 2 +- 4 files changed, 46 insertions(+), 20 deletions(-) diff --git a/include/flatbuffers/idl.h b/include/flatbuffers/idl.h index 5ebe4c9f6..262d21937 100644 --- a/include/flatbuffers/idl.h +++ b/include/flatbuffers/idl.h @@ -510,6 +510,8 @@ class Parser : public ParserState { // directory. // If the source was loaded from a file and isn't an include file, // supply its name in source_filename. + // All paths specified in this call must be in posix format, if you accept + // paths from user input, please call PosixPath on them first. bool Parse(const char *_source, const char **include_paths = nullptr, const char *source_filename = nullptr); diff --git a/include/flatbuffers/util.h b/include/flatbuffers/util.h index 870d2b60b..64bb231db 100644 --- a/include/flatbuffers/util.h +++ b/include/flatbuffers/util.h @@ -158,16 +158,20 @@ inline bool SaveFile(const char *name, const std::string &buf, bool binary) { return SaveFile(name, buf.c_str(), buf.size(), binary); } -// Functionality for minimalistic portable path handling: +// Functionality for minimalistic portable path handling. -static const char kPosixPathSeparator = '/'; -#ifdef _WIN32 -static const char kPathSeparator = '\\'; +// The functions below behave correctly regardless of whether posix ('/') or +// Windows ('/' or '\\') separators are used. + +// Any new separators inserted are always posix. + +// We internally store paths in posix format ('/'). Paths supplied +// by the user should go through PosixPath to ensure correct behavior +// on Windows when paths are string-compared. + +static const char kPathSeparator = '/'; +static const char kPathSeparatorWindows = '\\'; static const char *PathSeparatorSet = "\\/"; // Intentionally no ':' -#else -static const char kPathSeparator = kPosixPathSeparator; -static const char *PathSeparatorSet = "/"; -#endif // _WIN32 // Returns the path with the extension, if any, removed. inline std::string StripExtension(const std::string &filepath) { @@ -198,13 +202,24 @@ inline std::string StripFileName(const std::string &filepath) { inline std::string ConCatPathFileName(const std::string &path, const std::string &filename) { std::string filepath = path; - if (path.length() && path[path.size() - 1] != kPathSeparator && - path[path.size() - 1] != kPosixPathSeparator) - filepath += kPathSeparator; + if (filepath.length()) { + if (filepath.back() == kPathSeparatorWindows) { + filepath.back() = kPathSeparator; + } else if (filepath.back() != kPathSeparator) { + filepath += kPathSeparator; + } + } filepath += filename; return filepath; } +// Replaces any '\\' separators with '/' +inline std::string PosixPath(const char *path) { + std::string p = path; + std::replace(p.begin(), p.end(), '\\', '/'); + return p; +} + // This function ensure a directory exists, by recursively // creating dirs for any parts of the path that don't exist yet. inline void EnsureDirExists(const std::string &filepath) { diff --git a/src/flatc.cpp b/src/flatc.cpp index b791fb09b..40a14678d 100644 --- a/src/flatc.cpp +++ b/src/flatc.cpp @@ -16,6 +16,8 @@ #include "flatbuffers/flatc.h" +#include + #define FLATC_VERSION "1.6.0 (" __DATE__ ")" namespace flatbuffers { @@ -128,6 +130,7 @@ int FlatCompiler::Compile(int argc, const char** argv) { bool schema_binary = false; bool grpc_enabled = false; std::vector filenames; + std::list include_directories_storage; std::vector include_directories; std::vector conform_include_directories; std::vector generator_enabled(params_.num_generators, false); @@ -141,21 +144,27 @@ int FlatCompiler::Compile(int argc, const char** argv) { Error("invalid option location: " + arg, true); if (arg == "-o") { if (++argi >= argc) Error("missing path following: " + arg, true); - output_path = flatbuffers::ConCatPathFileName(argv[argi], ""); + output_path = flatbuffers::ConCatPathFileName( + flatbuffers::PosixPath(argv[argi]), ""); } else if(arg == "-I") { if (++argi >= argc) Error("missing path following" + arg, true); - include_directories.push_back(argv[argi]); + include_directories_storage.push_back( + flatbuffers::PosixPath(argv[argi])); + include_directories.push_back( + include_directories_storage.back().c_str()); } else if(arg == "--conform") { if (++argi >= argc) Error("missing path following" + arg, true); - conform_to_schema = argv[argi]; + conform_to_schema = flatbuffers::PosixPath(argv[argi]); } else if (arg == "--conform-includes") { if (++argi >= argc) Error("missing path following" + arg, true); - conform_include_directories.push_back(argv[argi]); + include_directories_storage.push_back( + flatbuffers::PosixPath(argv[argi])); + conform_include_directories.push_back( + include_directories_storage.back().c_str()); } else if (arg == "--include-prefix") { if (++argi >= argc) Error("missing path following" + arg, true); - opts.include_prefix = argv[argi]; - if (opts.include_prefix.back() != '/' && - opts.include_prefix.back() != '\\') opts.include_prefix += "/"; + opts.include_prefix = flatbuffers::ConCatPathFileName( + flatbuffers::PosixPath(argv[argi]), ""); } else if(arg == "--keep-prefix") { opts.keep_include_path = true; } else if(arg == "--strict-json") { @@ -240,7 +249,7 @@ int FlatCompiler::Compile(int argc, const char** argv) { found:; } } else { - filenames.push_back(argv[argi]); + filenames.push_back(flatbuffers::PosixPath(argv[argi])); } } diff --git a/src/idl_parser.cpp b/src/idl_parser.cpp index b813491ce..4c2556272 100644 --- a/src/idl_parser.cpp +++ b/src/idl_parser.cpp @@ -1962,7 +1962,7 @@ CheckedError Parser::DoParse(const char *source, const char **include_paths, Is(kTokenIdentifier))) { NEXT(); if (opts.proto_mode && attribute_ == "public") NEXT(); - auto name = attribute_; + auto name = flatbuffers::PosixPath(attribute_.c_str()); EXPECT(kTokenStringConstant); // Look for the file in include_paths. std::string filepath;