backend: fix --path_prefix validation

tensorboard/backend/application.py

@@ -252,10 +252,10 @@ def __init__(self, plugins, path_prefix=''):
     :type plugins: list[base_plugin.TBPlugin]
     """
     self._plugins = plugins
-    if path_prefix.endswith('/'):
-      self._path_prefix = path_prefix[:-1]
-    else:
-      self._path_prefix = path_prefix
+    self._path_prefix = path_prefix
+    if self._path_prefix.endswith('/'):
+      # Should have been fixed by `fix_flags`.
+      raise ValueError('Trailing slash in path prefix: %r' % self._path_prefix)
 
     self.exact_routes = {
         # TODO(@chihuahua): Delete this RPC once we have skylark rules that

tensorboard/plugins/core/core_plugin.py

@@ -435,9 +435,9 @@ def define_flags(self, parser):
 An optional, relative prefix to the path, e.g. "/path/to/tensorboard".
 resulting in the new base url being located at
 localhost:6006/path/to/tensorboard under default settings. A leading
-slash is required when specifying the path_prefix, however trailing
-slashes can be omitted. The path_prefix can be leveraged for path based
-routing of an elb when the website base_url is not available e.g.
+slash is required when specifying the path_prefix. A trailing slash is
+optional and has no effect. The path_prefix can be leveraged for path
+based routing of an ELB when the website base_url is not available e.g.
 "example.site.com/path/to/tensorboard/".\
 ''')
 
@@ -568,8 +568,10 @@ def fix_flags(self, flags):
     elif flags.host is not None and flags.bind_all:
       raise FlagsError('Must not specify both --host and --bind_all.')
 
-    if flags.path_prefix.endswith('/'):
-      flags.path_prefix = flags.path_prefix[:-1]
+    flags.path_prefix = flags.path_prefix.rstrip('/')
+    if flags.path_prefix and not flags.path_prefix.startswith('/'):
+      raise FlagsError(
+          'Path prefix must start with slash, but got: %r.' % flags.path_prefix)
 
   def load(self, context):
     """Creates CorePlugin instance."""

tensorboard/plugins/core/core_plugin_test.py

@@ -111,9 +111,25 @@ def testFlag(self):
     with six.assertRaisesRegex(self, ValueError, logdir_or_db_req):
       loader.fix_flags(FakeFlags(inspect=False, event_file='/tmp/event.out'))
 
-    flag = FakeFlags(inspect=False, logdir='/tmp', path_prefix='hello/')
-    loader.fix_flags(flag)
-    self.assertEqual(flag.path_prefix, 'hello')
+  def testPathPrefix_stripsTrailingSlashes(self):
+    loader = core_plugin.CorePluginLoader()
+    for path_prefix in ('/hello', '/hello/', '/hello//', '/hello///'):
+      flag = FakeFlags(inspect=False, logdir='/tmp', path_prefix=path_prefix)
+      loader.fix_flags(flag)
+      self.assertEqual(
+          flag.path_prefix,
+          '/hello',
+          'got %r (input %r)' % (flag.path_prefix, path_prefix),
+      )
+
+  def testPathPrefix_mustStartWithSlash(self):
+    loader = core_plugin.CorePluginLoader()
+    flag = FakeFlags(inspect=False, logdir='/tmp', path_prefix='noslash')
+    with self.assertRaises(base_plugin.FlagsError) as cm:
+      loader.fix_flags(flag)
+    msg = str(cm.exception)
+    self.assertIn('must start with slash', msg)
+    self.assertIn(repr('noslash'), msg)
 
   def testIndex_returnsActualHtml(self):
     """Test the format of the /data/runs endpoint."""