Add more tests for exclude validation

cucumber.js

@@ -3,5 +3,5 @@ module.exports = {
     verify: '--strict --tags ~@stress --tags ~@mld --tags ~@todo -f progress --require features/support --require features/step_definitions',
     todo: '--strict --tags @todo --require features/support --require features/step_definitions',
     all: '--strict --require features/support --require features/step_definitions',
-    mld: '--strict --tags ~@stress --tags ~@todo --tags ~@alternative --require features/support --require features/step_definitions -f progress'
+    mld: '--strict --tags ~@stress --tags ~@todo --tags ~@alternative --tags ~@ch --require features/support --require features/step_definitions -f progress'
 };

features/testbot/exclude.feature

@@ -1,4 +1,4 @@
-@routing @testbot @exclude @mld
+@routing @testbot @exclude
 Feature: Testbot - Exclude flags
     Background:
         Given the profile "testbot"
@@ -20,6 +20,7 @@ Feature: Testbot - Exclude flags
             | fg    | primary  |      | always drivable                                                          |
             | gd    | primary  |      | always drivable                                                          |
 
+    @mld
     Scenario: Testbot - exclude nothing
         When I route I should get
             | from | to | route          |
@@ -37,6 +38,7 @@ Feature: Testbot - Exclude flags
             | a | 0   | 115 |
             | d | 115 | 0   |
 
+    @mld
     Scenario: Testbot - exclude motorway
         Given the query options
             | exclude  | motorway        |
@@ -57,6 +59,7 @@ Feature: Testbot - Exclude flags
             | a | 0   | 125 |
             | d | 125 | 0  |
 
+    @mld
     Scenario: Testbot - exclude toll
         Given the query options
             | exclude | toll |
@@ -69,6 +72,7 @@ Feature: Testbot - Exclude flags
             | a    | f  |          |
             | f    | d  | fg,gd,gd |
 
+    @mld
     Scenario: Testbot - exclude motorway and toll
         Given the query options
             | exclude | motorway,toll |
@@ -81,4 +85,30 @@ Feature: Testbot - Exclude flags
             | a    | f  |          |
             | f    | d  | fg,gd,gd |
 
+    @mld
+    Scenario: Testbot - exclude with unsupported exclude combination
+        Given the query options
+            | exclude | TwoWords2 |
+
+        When I route I should get
+            | from | to | status | message   |
+            | a    | d  | 400    | Exclude flag combination is not supported. |
+
+    @mld
+    Scenario: Testbot - exclude with invalid exclude class name
+        Given the query options
+            | exclude | foo |
+
+        When I route I should get
+            | from | to | status | message   |
+            | a    | d  | 400    | Exclude flag combination is not supported. |
+
+    @ch
+    Scenario: Testbot - Check error message for exclude on non-MLD
+        Given the query options
+            | exclude | motorway |
+
+        When I route I should get
+            | from | to | status | message   |
+            | a    | d  | 400    | This algorithm does not support exclude flags. |
 

include/extractor/class_data.hpp

@@ -3,6 +3,8 @@
 
 #include "util/bit_range.hpp"
 
+#include <algorithm>
+#include <cctype>
 #include <cstdint>
 
 namespace osrm
@@ -24,6 +26,13 @@ inline auto getClassData(const std::size_t index)
     BOOST_ASSERT(index <= MAX_CLASS_INDEX);
     return uint8_t{1} << index;
 }
+
+inline bool isValidClassName(const std::string &name)
+{
+    return std::find_if_not(name.begin(), name.end(), [](const auto c) {
+               return std::isalnum(c);
+           }) == name.end();
+}
 }
 }
 

include/server/api/base_parameters_grammar.hpp

@@ -163,7 +163,7 @@ struct BaseParametersGrammar : boost::spirit::qi::grammar<Iterator, Signature>
                          ';')[ph::bind(&engine::api::BaseParameters::approaches, qi::_r1) = qi::_1];
 
         exclude_rule = qi::lit("exclude=") >
-                       (qi::as_string[+qi::char_("a-zA-Z")] %
+                       (qi::as_string[+qi::char_("a-zA-Z0-9")] %
                         ',')[ph::bind(&engine::api::BaseParameters::exclude, qi::_r1) = qi::_1];
 
         base_rule = radiuses_rule(qi::_r1)         //

profiles/testbot.lua

@@ -19,7 +19,7 @@ function setup()
       use_turn_restrictions         = true
     },
 
-    classes = {[0] = "motorway", [1] = "toll"},
+    classes = {[0] = "motorway", [1] = "toll", [2] = "TooWords2"},
 
     excludable = {
         [1] = {["motorway"] = true},

src/extractor/extractor.cpp

@@ -80,6 +80,12 @@ void SetClassNames(const std::vector<std::string> &class_names,
         // this makes sure we can correctly validate unkown class names later
         for (const auto &name : class_names)
         {
+            if (!isValidClassName(name))
+            {
+                throw util::exception("Invalid class name " + name +
+                                      " only [a-Z0-9] allowed.");
+            }
+
             auto iter = classes_map.find(name);
             if (iter == classes_map.end())
             {

src/extractor/extractor_callbacks.cpp

@@ -202,6 +202,12 @@ void ExtractorCallbacks::ProcessWay(const osmium::Way &input_way, const Extracti
         ClassData mask = 0;
         for (const auto &name_and_flag : classes)
         {
+            if (!isValidClassName(name_and_flag.first))
+            {
+                throw util::exception("Invalid class name " + name_and_flag.first +
+                                      " only [a-Z0-9] allowed.");
+            }
+
             if (name_and_flag.second)
             {
                 mask |= classStringToMask(name_and_flag.first);