From 72485ddbf4e0c8a57201fe27f1dc69502542e01f Mon Sep 17 00:00:00 2001
From: Simon Lydell <simon.lydell@gmail.com>
Date: Thu, 17 Nov 2016 19:29:08 +0100
Subject: [PATCH] Validate all config file API method arguments

This provides a better user experience and also prevents VimFx from
crashing due to invalid arguments.

Fixes #837.
---
 extension/lib/api.coffee       | 60 +++++++++++++++++++--
 extension/test/test-api.coffee | 98 +++++++++++++++++++++++++++++++++-
 2 files changed, 153 insertions(+), 5 deletions(-)

diff --git a/extension/lib/api.coffee b/extension/lib/api.coffee
index df837d8..02b0371 100644
--- a/extension/lib/api.coffee
+++ b/extension/lib/api.coffee
@@ -107,6 +107,12 @@ createConfigAPI = (vimfx, {allowDeprecated = true} = {}) -> {
     onShutdown(vimfx, -> delete vimfx.modes[mode].commands[name])
 
   addOptionOverrides: (rules...) ->
+    validateRules(rules, (override) ->
+      unless Object::toString.call(override) == '[object Object]'
+        return 'an object'
+      return null
+    )
+
     unless vimfx.optionOverrides
       vimfx.optionOverrides = []
       vimfx.options = new Proxy(vimfx.options, {
@@ -117,9 +123,17 @@ createConfigAPI = (vimfx, {allowDeprecated = true} = {}) -> {
           return overrides?[pref] ? options[pref]
       })
       onShutdown(vimfx, -> vimfx.optionOverrides = [])
+
     vimfx.optionOverrides.push(rules...)
 
   addKeyOverrides: (rules...) ->
+    validateRules(rules, (override) ->
+      unless Array.isArray(override) and
+             override.every((item) -> typeof item == 'string')
+        return 'an array of strings'
+      return null
+    )
+
     unless vimfx.keyOverrides
       vimfx.keyOverrides = []
       vimfx.options.keyValidator = (keyStr, mode) ->
@@ -129,6 +143,7 @@ createConfigAPI = (vimfx, {allowDeprecated = true} = {}) -> {
         overrides = getOverrides(vimfx.keyOverrides, location)
         return keyStr not in (overrides ? [])
       onShutdown(vimfx, -> vimfx.keyOverrides = [])
+
     vimfx.keyOverrides.push(rules...)
 
   send: (vim, message, data = null, callback = null) ->
@@ -152,10 +167,14 @@ createConfigAPI = (vimfx, {allowDeprecated = true} = {}) -> {
     vim._send(message, data, callback, {prefix: 'config:'})
 
   on: (event, listener) ->
+    validateEventListener(event, listener)
     vimfx.on(event, listener)
     onShutdown(vimfx, -> vimfx.off(event, listener))
 
-  off: vimfx.off.bind(vimfx)
+  off: (event, listener) ->
+    validateEventListener(event, listener)
+    vimfx.off(event, listener)
+
   modes: vimfx.modes
 }
 
@@ -185,10 +204,45 @@ renamedPrefs = {
 }
 
 getOverrides = (rules, args...) ->
-  for [match, overrides] in rules
-    return overrides if match(args...)
+  for [matcher, override] in rules
+    return override if matcher(args...)
   return null
 
+validateRules = (rules, overrideValidator) ->
+  for rule in rules
+    unless Array.isArray(rule)
+      throw new Error(
+        "VimFx: An override rule must be an array. Got: #{rule}"
+      )
+    unless rule.length == 2
+      throw new Error(
+        "VimFx: An override rule array must be of length 2. Got: #{rule.length}"
+      )
+    [matcher, override] = rule
+    unless typeof matcher == 'function'
+      throw new Error(
+        "VimFx: The first item of an override rule array must be a function.
+         Got: #{matcher}"
+      )
+    overrideValidationMessage = overrideValidator(override)
+    if overrideValidationMessage
+      throw new Error(
+        "VimFx: The second item of an override rule array must be
+         #{overrideValidationMessage}. Got: #{override}"
+      )
+  return
+
+validateEventListener = (event, listener) ->
+  unless typeof event == 'string'
+    throw new Error(
+      "VimFx: The first argument must be a string. Got: #{event}"
+    )
+  unless typeof listener == 'function'
+    throw new Error(
+      "VimFx: The second argument must be a function. Got: #{listener}"
+    )
+  return
+
 onShutdown = (vimfx, handler) ->
   fn = ->
     handler()
diff --git a/extension/test/test-api.coffee b/extension/test/test-api.coffee
index 28f2f32..24729d3 100644
--- a/extension/test/test-api.coffee
+++ b/extension/test/test-api.coffee
@@ -213,7 +213,7 @@ exports['test vimfx.addOptionOverrides'] = (assert, $vimfx, teardown) ->
   vimfx = createConfigAPI($vimfx)
 
   originalOptions = Object.assign({}, $vimfx.options)
-  originalOptionOverrides = Object.assign({}, $vimfx.optionOverrides)
+  originalOptionOverrides = $vimfx.optionOverrides?[..]
   $vimfx.optionOverrides = null
   $vimfx.options.prevent_autofocus = true
   teardown(->
@@ -244,7 +244,7 @@ exports['test vimfx.addKeyOverrides'] = (assert, $vimfx, teardown) ->
   vimfx = createConfigAPI($vimfx)
 
   originalOptions = Object.assign({}, $vimfx.options)
-  originalKeyOverrides = Object.assign({}, $vimfx.keyOverrides)
+  originalKeyOverrides = $vimfx.keyOverrides?[..]
   $vimfx.options.keyValidator = null
   $vimfx.options.ignore_keyboard_layout = false
   $vimfx.options.translations = {}
@@ -476,6 +476,100 @@ exports['test vimfx.addCommand errors'] = (assert, $vimfx) ->
     vimfx.addCommand({name: 'test_command', description: 'Test command'}, false)
   )
 
+exports['test vimfx.add{Option,Key}Overrides errors'] = (assert, $vimfx) ->
+  vimfx = createConfigAPI($vimfx)
+
+  # Passing nothing is OK, and just shouldn’t throw.
+  vimfx.addOptionOverrides()
+  vimfx.addKeyOverrides()
+
+  throws(assert, /array/i, '1', ->
+    vimfx.addOptionOverrides(1)
+  )
+
+  throws(assert, /array/i, '1', ->
+    vimfx.addKeyOverrides(1)
+  )
+
+  throws(assert, /length 2/i, '0', ->
+    vimfx.addOptionOverrides([])
+  )
+
+  throws(assert, /length 2/i, '0', ->
+    vimfx.addKeyOverrides([])
+  )
+
+  throws(assert, /length 2/i, '1', ->
+    vimfx.addOptionOverrides([1])
+  )
+
+  throws(assert, /length 2/i, '1', ->
+    vimfx.addKeyOverrides([1])
+  )
+
+  throws(assert, /length 2/i, '3', ->
+    vimfx.addOptionOverrides([1, 2, 3])
+  )
+
+  throws(assert, /length 2/i, '3', ->
+    vimfx.addKeyOverrides([1, 2, 3])
+  )
+
+  throws(assert, /function/i, 'null', ->
+    vimfx.addOptionOverrides([null, 2])
+  )
+
+  throws(assert, /function/i, 'null', ->
+    vimfx.addKeyOverrides([null, 2])
+  )
+
+  throws(assert, /object/i, 'null', ->
+    vimfx.addOptionOverrides([(-> true), null])
+  )
+
+  throws(assert, /array of strings/i, '[object Object]', ->
+    vimfx.addKeyOverrides([(-> true), {j: false}])
+  )
+
+  throws(assert, /array of strings/i, '1,2', ->
+    vimfx.addKeyOverrides([(-> true), [1, 2]])
+  )
+
+exports['test vimfx.{on,off} errors'] = (assert, $vimfx) ->
+  vimfx = createConfigAPI($vimfx)
+
+  throws(assert, /string/i, 'undefined', ->
+    vimfx.on()
+  )
+
+  throws(assert, /string/i, 'undefined', ->
+    vimfx.off()
+  )
+
+  throws(assert, /string/i, '1', ->
+    vimfx.on(1)
+  )
+
+  throws(assert, /string/i, '1', ->
+    vimfx.off(1)
+  )
+
+  throws(assert, /function/i, 'undefined', ->
+    vimfx.on('event')
+  )
+
+  throws(assert, /function/i, 'undefined', ->
+    vimfx.off('event')
+  )
+
+  throws(assert, /function/i, 'null', ->
+    vimfx.on('event', null)
+  )
+
+  throws(assert, /function/i, 'null', ->
+    vimfx.off('event', null)
+  )
+
 exports['test vimfx.send errors'] = (assert, $vimfx) ->
   vimfx = createConfigAPI($vimfx)
 
-- 
2.39.3