[Boost-commit] svn:boost r86704 - in trunk/tools/quickbook: src test

Subject: [Boost-commit] svn:boost r86704 - in trunk/tools/quickbook: src test
From: dnljms_at_[hidden]
Date: 2013-11-14 14:22:48

Author: danieljames
Date: 2013-11-14 14:22:48 EST (Thu, 14 Nov 2013)
New Revision: 86704
URL: http://svn.boost.org/trac/boost/changeset/86704

Log:
Error for invalid paths containing escaped slashes.

Note that the test cases are really awkward so this is unlikely to
actually happen normally, but implementation is much easier if it
doesn't have to worry about these cases.

Added:
   trunk/tools/quickbook/test/include_invalid_path1-1_7-fail.quickbook   (contents, props changed)
   trunk/tools/quickbook/test/include_invalid_path2-1_7-fail.quickbook   (contents, props changed)
Text files modified: 
   trunk/tools/quickbook/src/actions.cpp                               |    53 +++++++----                             
   trunk/tools/quickbook/src/include_paths.cpp                         |   189 ++++++++++++++++++++++----------------- 
   trunk/tools/quickbook/src/include_paths.hpp                         |     2                                         
   trunk/tools/quickbook/test/Jamfile.v2                               |     2                                         
   trunk/tools/quickbook/test/include_invalid_path1-1_7-fail.quickbook |     5 +                                       
   trunk/tools/quickbook/test/include_invalid_path2-1_7-fail.quickbook |     5 +                                       
   6 files changed, 155 insertions(+), 101 deletions(-)

Modified: trunk/tools/quickbook/src/actions.cpp
==============================================================================
--- trunk/tools/quickbook/src/actions.cpp	Thu Nov 14 14:22:09 2013	(r86703)
+++ trunk/tools/quickbook/src/actions.cpp	2013-11-14 14:22:48 EST (Thu, 14 Nov 2013)	(r86704)
@@ -1863,30 +1863,45 @@
     {
         path_parameter parameter = check_path(p, state);
 
-        if (parameter.type == path_parameter::glob) {
-            // TODO: Should know if this is an xinclude or an xmlbase.
-            // Would also help with implementation of 'check_path'.
-            detail::outerr(p.get_file(), p.get_position())
-                << "Glob used in xinclude/xmlbase."
-                << std::endl;
-            ++state.error_count;
-            return xinclude_path(state.current_file->path.parent_path(), "");
-        }
+        switch (parameter.type) {
+            case path_parameter::glob:
+                // TODO: Should know if this is an xinclude or an xmlbase.
+                // Would also help with implementation of 'check_path'.
+                detail::outerr(p.get_file(), p.get_position())
+                    << "Glob used in xinclude/xmlbase."
+                    << std::endl;
+                ++state.error_count;
+                break;
 
-        fs::path path = detail::generic_to_path(parameter.value);
-        fs::path full_path = path;
+            case path_parameter::invalid:
+                // There should have already been an error message in this case.
+                break;
 
-        // If the path is relative
-        if (!path.has_root_directory())
-        {
-            // Resolve the path from the current file
-            full_path = state.current_file->path.parent_path() / path;
+            case path_parameter::path:
+            {
+                fs::path path = detail::generic_to_path(parameter.value);
+                fs::path full_path = path;
+
+                // If the path is relative
+                if (!path.has_root_directory())
+                {
+                    // Resolve the path from the current file
+                    full_path = state.current_file->path.parent_path() / path;
+
+                    // Then calculate relative to the current xinclude_base.
+                    path = path_difference(state.xinclude_base, full_path);
+                }
+
+                return xinclude_path(full_path,
+                        detail::escape_uri(detail::path_to_generic(path)));
+            }
 
-            // Then calculate relative to the current xinclude_base.
-            path = path_difference(state.xinclude_base, full_path);
+            default:
+                assert(false);
         }
 
-        return xinclude_path(full_path, detail::escape_uri(detail::path_to_generic(path)));
+        // If we didn't find a path, just use this:
+        return xinclude_path(state.current_file->path.parent_path(), "");
     }
 
     void xinclude_action(quickbook::state& state, value xinclude)

Modified: trunk/tools/quickbook/src/include_paths.cpp
==============================================================================
--- trunk/tools/quickbook/src/include_paths.cpp	Thu Nov 14 14:22:09 2013	(r86703)
+++ trunk/tools/quickbook/src/include_paths.cpp	2013-11-14 14:22:48 EST (Thu, 14 Nov 2013)	(r86704)
@@ -28,40 +28,57 @@
 
     path_parameter check_path(value const& path, quickbook::state& state)
     {
-        // Paths are encoded for quickbook 1.6+ and also xmlbase
-        // values (technically xmlbase is a 1.6 feature, but that
-        // isn't enforced as it's backwards compatible).
-        //
-        // Counter-intuitively: encoded == plain text here.
+        if (qbk_version_n >= 107u) {
+            std::string path_text = path.get_encoded();
 
-        std::string path_text = qbk_version_n >= 106u || path.is_encoded() ?
-                path.get_encoded() : detail::to_s(path.get_quickbook());
-
-        bool is_glob = qbk_version_n >= 107u &&
-            path_text.find_first_of("[]?*") != std::string::npos;
-
-        if(!is_glob && path_text.find('\\') != std::string::npos)
-        {
-            quickbook::detail::ostream* err;
-
-            if (qbk_version_n >= 106u) {
-                err = &detail::outerr(path.get_file(), path.get_position());
+            if (path_text.find("\\\\") != std::string::npos ||
+                    path_text.find("\\/") != std::string::npos)
+            {
+                detail::outerr(path.get_file(), path.get_position())
+                    << "Invalid path (contains escaped slash): "
+                    << path_text
+                   << std::endl;
                 ++state.error_count;
-            }
-            else {
-                err = &detail::outwarn(path.get_file(), path.get_position());
+                return path_parameter(path_text, path_parameter::invalid);
             }
 
-            *err << "Path isn't portable: '"
-                << path_text
-                << "'"
-                << std::endl;
+            bool is_glob = path_text.find_first_of("[]?*") != std::string::npos;
 
-            boost::replace(path_text, '\\', '/');
+            return path_parameter(path_text,
+                is_glob ? path_parameter::glob : path_parameter::path);
         }
+        else {
+            // Paths are encoded for quickbook 1.6+ and also xmlbase
+            // values (technically xmlbase is a 1.6 feature, but that
+            // isn't enforced as it's backwards compatible).
+            //
+            // Counter-intuitively: encoded == plain text here.
 
-        return path_parameter(path_text,
-            is_glob ? path_parameter::glob : path_parameter::path);
+            std::string path_text = qbk_version_n >= 106u || path.is_encoded() ?
+                    path.get_encoded() : detail::to_s(path.get_quickbook());
+
+            if (path_text.find('\\') != std::string::npos)
+            {
+                quickbook::detail::ostream* err;
+
+                if (qbk_version_n >= 106u) {
+                    err = &detail::outerr(path.get_file(), path.get_position());
+                    ++state.error_count;
+                }
+                else {
+                    err = &detail::outwarn(path.get_file(), path.get_position());
+                }
+
+                *err << "Path isn't portable: '"
+                    << path_text
+                    << "'"
+                    << std::endl;
+
+                boost::replace(path_text, '\\', '/');
+            }
+
+            return path_parameter(path_text, path_parameter::path);
+        }
     }
 
     //
@@ -148,76 +165,86 @@
     {
         std::set<quickbook_path> result;
 
-        // If the path has some glob match characters
-        // we do a discovery of all the matches..
-        if (parameter.type == path_parameter::glob)
-        {
-            fs::path current = state.current_file->path.parent_path();
+        switch (parameter.type) {
+            case path_parameter::glob:
+            // If the path has some glob match characters
+            // we do a discovery of all the matches..
+            {
+                fs::path current = state.current_file->path.parent_path();
+
+                // Search for the current dir accumulating to the result.
+                state.dependencies.add_glob(current / parameter.value);
+                include_search_glob(result,
+                        quickbook_path(current,
+                            state.abstract_file_path.parent_path()),
+                        parameter.value, state);
 
-            // Search for the current dir accumulating to the result.
-            state.dependencies.add_glob(current / parameter.value);
-            include_search_glob(result,
-                    quickbook_path(current,
-                        state.abstract_file_path.parent_path()),
-                    parameter.value, state);
+                // Search the include path dirs accumulating to the result.
+                BOOST_FOREACH(fs::path dir, include_path)
+                {
+                    state.dependencies.add_glob(dir / parameter.value);
+                    include_search_glob(result, quickbook_path(dir, fs::path()),
+                            parameter.value, state);
+                }
 
-            // Search the include path dirs accumulating to the result.
-            BOOST_FOREACH(fs::path dir, include_path)
-            {
-                state.dependencies.add_glob(dir / parameter.value);
-                include_search_glob(result, quickbook_path(dir, fs::path()),
-                        parameter.value, state);
+                // Done.
+                return result;
             }
 
-            // Done.
-            return result;
-        }
-        else
-        {
-            fs::path path = detail::generic_to_path(parameter.value);
-
-            // If the path is relative, try and resolve it.
-            if (!path.has_root_directory() && !path.has_root_name())
+            case path_parameter::path:
             {
-                fs::path local_path =
-                   state.current_file->path.parent_path() / path;
-
-                // See if it can be found locally first.
-                if (state.dependencies.add_dependency(local_path))
-                {
-                    result.insert(quickbook_path(
-                        local_path,
-                        state.abstract_file_path.parent_path() / path));
-                    return result;
-                }
+                fs::path path = detail::generic_to_path(parameter.value);
 
-                // Search in each of the include path locations.
-                BOOST_FOREACH(fs::path full, include_path)
+                // If the path is relative, try and resolve it.
+                if (!path.has_root_directory() && !path.has_root_name())
                 {
-                    full /= path;
+                    fs::path local_path =
+                        state.current_file->path.parent_path() / path;
 
-                    if (state.dependencies.add_dependency(full))
+                    // See if it can be found locally first.
+                    if (state.dependencies.add_dependency(local_path))
                     {
-                        result.insert(quickbook_path(full, path));
+                        result.insert(quickbook_path(
+                                    local_path,
+                                    state.abstract_file_path.parent_path() / path));
                         return result;
                     }
+
+                    // Search in each of the include path locations.
+                    BOOST_FOREACH(fs::path full, include_path)
+                    {
+                        full /= path;
+
+                        if (state.dependencies.add_dependency(full))
+                        {
+                            result.insert(quickbook_path(full, path));
+                            return result;
+                        }
+                    }
                 }
-            }
-            else
-            {
-                if (state.dependencies.add_dependency(path)) {
-                    result.insert(quickbook_path(path, path));
-                    return result;
+                else
+                {
+                    if (state.dependencies.add_dependency(path)) {
+                        result.insert(quickbook_path(path, path));
+                        return result;
+                    }
                 }
+
+                detail::outerr(state.current_file, pos)
+                    << "Unable to find file: "
+                    << parameter.value
+                    << std::endl;
+                ++state.error_count;
+
+                return result;
             }
 
-            detail::outerr(state.current_file, pos)
-                << "Unable to find file: "
-                << parameter.value
-                << std::endl;
-            ++state.error_count;
+            case path_parameter::invalid:
+                return result;
 
-            return result;
+            default:
+                assert(0);
+                return result;
         }
     }
 

Modified: trunk/tools/quickbook/src/include_paths.hpp
==============================================================================
--- trunk/tools/quickbook/src/include_paths.hpp	Thu Nov 14 14:22:09 2013	(r86703)
+++ trunk/tools/quickbook/src/include_paths.hpp	2013-11-14 14:22:48 EST (Thu, 14 Nov 2013)	(r86704)
@@ -25,7 +25,7 @@
 {
     struct path_parameter {
         // Will possibly add 'url' to this list later:
-        enum path_type { path, glob };
+        enum path_type { invalid, path, glob };
 
         std::string value;
         path_type type;

Modified: trunk/tools/quickbook/test/Jamfile.v2
==============================================================================
--- trunk/tools/quickbook/test/Jamfile.v2	Thu Nov 14 14:22:09 2013	(r86703)
+++ trunk/tools/quickbook/test/Jamfile.v2	2013-11-14 14:22:48 EST (Thu, 14 Nov 2013)	(r86704)
@@ -68,6 +68,8 @@
     [ quickbook-test include-1_7 ]
     [ quickbook-test include2-1_6 ]
     [ quickbook-error-test include_win_path-1_6-fail ]
+    [ quickbook-error-test include_invalid_path1-1_7-fail ]
+    [ quickbook-error-test include_invalid_path2-1_7-fail ]
     [ quickbook-test link-1_1 ]
     [ quickbook-test link-1_6 ]
     [ quickbook-test link-1_7 ]

Added: trunk/tools/quickbook/test/include_invalid_path1-1_7-fail.quickbook
==============================================================================
--- /dev/null	00:00:00 1970	(empty, because file is newly added)
+++ trunk/tools/quickbook/test/include_invalid_path1-1_7-fail.quickbook	2013-11-14 14:22:48 EST (Thu, 14 Nov 2013)	(r86704)
@@ -0,0 +1,5 @@
+[article Include invalid path fail
+[quickbook 1.7]
+]
+
+[include .\\\/empty-inc.quickbook]

Added: trunk/tools/quickbook/test/include_invalid_path2-1_7-fail.quickbook
==============================================================================
--- /dev/null	00:00:00 1970	(empty, because file is newly added)
+++ trunk/tools/quickbook/test/include_invalid_path2-1_7-fail.quickbook	2013-11-14 14:22:48 EST (Thu, 14 Nov 2013)	(r86704)
@@ -0,0 +1,5 @@
+[article Include invalid path fail
+[quickbook 1.7]
+]
+
+[include .\\\\empty-inc.quickbook]