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

@@ -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);
 

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) {

src/flatc.cpp

@@ -16,6 +16,8 @@
 
 #include "flatbuffers/flatc.h"
 
+#include <list>
+
 #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<std::string> filenames;
+  std::list<std::string> include_directories_storage;
   std::vector<const char *> include_directories;
   std::vector<const char *> conform_include_directories;
   std::vector<bool> 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]));
     }
   }
 

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;