Raise a validation error for invalid RegExp in regex condition. Fix #36.

FelisCatus · Oct 3, 2014 · 3465f34 · 3465f34
1 parent 2913091
commit 3465f34
Show file tree

Hide file tree

Showing 4 changed files with 30 additions and 9 deletions.
diff --git a/omega-pac/src/conditions.coffee b/omega-pac/src/conditions.coffee
@@ -38,10 +38,17 @@ module.exports = exports =
     node.start.comments_before.push {type: 'comment2', value: comment}
     node
 
+  safeRegex: (expr) ->
+    try
+      new RegExp(expr)
+    catch
+      # Invalid regexp! Fall back to a regexp that does not match anything.
+      /(?!)/
+
   regTest: (expr, regexp) ->
     if typeof regexp == 'string'
       # Escape (unescaped) forward slash for use in regex literals.
-      regexp = new RegExp escapeSlash regexp
+      regexp = regexSafe escapeSlash regexp
     if typeof expr == 'string'
       expr = new U2.AST_SymbolRef name: expr
     new U2.AST_Call
@@ -164,7 +171,7 @@ module.exports = exports =
       compile: (condition) -> new U2.AST_False
     'UrlRegexCondition':
       tag: (condition) -> condition.pattern
-      analyze: (condition) -> new RegExp escapeSlash condition.pattern
+      analyze: (condition) -> @safeRegex escapeSlash condition.pattern
       match: (condition, request, cache) ->
         return cache.analyzed.test(request.url)
       compile: (condition, cache) ->
@@ -175,15 +182,15 @@ module.exports = exports =
       analyze: (condition) ->
         parts = for pattern in condition.pattern.split('|') when pattern
           shExp2RegExp pattern, trimAsterisk: true
-        new RegExp parts.join('|')
+        @safeRegex parts.join('|')
       match: (condition, request, cache) ->
         return cache.analyzed.test(request.url)
       compile: (condition, cache) ->
         @regTest 'url', cache.analyzed
 
     'HostRegexCondition':
       tag: (condition) -> condition.pattern
-      analyze: (condition) -> new RegExp escapeSlash condition.pattern
+      analyze: (condition) -> @safeRegex escapeSlash condition.pattern
       match: (condition, request, cache) ->
         return cache.analyzed.test(request.host)
       compile: (condition, cache) ->
@@ -206,7 +213,7 @@ module.exports = exports =
               .replace(/./, '(?:^|\\.)')
           else
             shExp2RegExp pattern, trimAsterisk: true
-        new RegExp parts.join('|')
+        @safeRegex parts.join('|')
       match: (condition, request, cache) ->
         return cache.analyzed.test(request.host)
       compile: (condition, cache) ->
@@ -261,14 +268,14 @@ module.exports = exports =
               serverRegex = shExp2RegExp(server)
               serverRegex = serverRegex.substring(1, serverRegex.length - 1)
             scheme = cache.scheme ? '[^:]+'
-            cache.url = new RegExp('^' + scheme + ':\\/\\/' + serverRegex +
+            cache.url = @safeRegex('^' + scheme + ':\\/\\/' + serverRegex +
               ':' + matchPort + '\\/')
           else if server != '*'
             if serverRegex
               serverRegex = '^' + serverRegex + '$'
             else
               serverRegex = shExp2RegExp server, trimAsterisk: true
-            cache.host = new RegExp serverRegex
+            cache.host = @safeRegex(serverRegex)
         return cache
       match: (condition, request, cache) ->
         cache = cache.analyzed

diff --git a/omega-pac/test/conditions.coffee b/omega-pac/test/conditions.coffee
@@ -61,7 +61,12 @@ describe 'Conditions', ->
       con =
         conditionType: 'UrlRegexCondition'
         pattern: 'exam.*\\.com'
-      testCond(cond, 'http://www.example.com/', 'match')
+      testCond(con, 'http://www.example.com/', 'match')
+    it 'should fallback to not match if pattern is invalid', ->
+      con =
+        conditionType: 'UrlRegexCondition'
+        pattern: ')Invalid('
+      testCond(con, 'http://www.example.com/', not 'match')
   describe 'UrlWildcardCondition', ->
     cond =
       conditionType: 'UrlWildcardCondition'

diff --git a/omega-web/src/omega/controllers/switch_profile.coffee b/omega-web/src/omega/controllers/switch_profile.coffee
@@ -21,6 +21,14 @@ angular.module('omega').controller 'SwitchProfileCtrl', ($scope, $modal) ->
       rule.condition.pattern = ''
     $scope.profile.rules.push rule
 
+  $scope.validateCondition = (condition, pattern) ->
+    if condition.conditionType.indexOf('Regex') >= 0
+      try
+        new RegExp(pattern)
+      catch
+        return false
+    return true
+
   $scope.removeRule = (index) ->
     removeForReal = ->
       $scope.profile.rules.splice index, 1

diff --git a/omega-web/src/partials/profile_switch.jade b/omega-web/src/partials/profile_switch.jade
@@ -25,7 +25,8 @@ section.settings-group(ng-controller='SwitchProfileCtrl')
               span {{'options_hostLevelsBetween' | tr}}
               = ' '
               input.form-control(type='number' max='99' min='1' ng-model='rule.condition.maxValue' required)
-            input.form-control(ng-model='rule.condition.pattern' ng-switch-default required)
+            input.form-control(ng-model='rule.condition.pattern' ng-switch-default required
+              ui-validate='{pattern: "validateCondition(rule.condition, $value)"}')
           td
             div.form-control(omega-profile-select='options | profiles:profile' ng-model='rule.profileName'
               disp-name='$profile.name | dispName')