Update API router code to throw a more user-friendly exception in case

the request URL contains invalid or incorrectly URL encoded characters.

Previously we didn't handle this scenario which means original
UnicodeDecodeError error with the stack trace got propagated to the user
which is a no go.

Related issue -
GoogleCloudPlatform/webapp2#152.

CHANGELOG.rst

@@ -28,6 +28,15 @@ Fixed
 
   Contributed by @Kami.
 
+* Fix a bug in the API router code and make sure we return correct and user-friendly error to the
+  user in case we fail to parse the request URL / path because it contains invalid or incorrectly
+  URL encoded data.
+
+  Previously such errors weren't handled correctly which meant original exception with a stack
+  trace got propagated to the user. (bug fix) #5189
+
+  Contributed by @Kami.
+
 Changed
 ~~~~~~~
 

st2common/st2common/middleware/instrumentation.py

@@ -20,6 +20,8 @@
 from st2common.metrics.base import get_driver
 from st2common.util.date import get_datetime_utc_now
 from st2common.router import NotFoundException
+from st2common.router import Response
+from st2common.util.jsonify import json_encode
 
 __all__ = ["RequestInstrumentationMiddleware", "ResponseInstrumentationMiddleware"]
 
@@ -47,6 +49,16 @@ def __call__(self, environ, start_response):
             endpoint, _ = self.router.match(request)
         except NotFoundException:
             endpoint = {}
+        except Exception as e:
+            # Special case to make sure we return friendly error to the user.
+            # If we don't do that and router.match() throws an exception, we will return stack trace
+            # to the end user which is not good.
+            status_code = getattr(e, "status_code", 500)
+            headers = {"Content-Type": "application/json"}
+            body = {"faultstring": getattr(e, "detail", str(e))}
+            response_body = json_encode(body)
+            resp = Response(response_body, status=status_code, headers=headers)
+            return resp(environ, start_response)
 
         # NOTE: We don't track per request and response metrics for /v1/executions/<id> and some
         # other endpoints because this would result in a lot of unique metrics which is an

st2common/st2common/router.py

@@ -242,7 +242,28 @@ def match(self, req):
         # NOTE: webob.url_unquote doesn't work correctly under Python 3 when paths contain non-ascii
         # characters. That method supposed to handle Python 2 and Python 3 compatibility, but it
         # doesn't work correctly under Python 3.
-        path = urllib.parse.unquote(req.path)
+        try:
+            path = urllib.parse.unquote(req.path)
+        except Exception as e:
+            # This exception being thrown indicates that the URL / path contains bad or incorrectly
+            # URL escaped characters. Instead of returning this stack track + 500 error to the
+            # user we return a friendly and more correct exception
+            # NOTE: We should not access or log req.path here since it's a property which results
+            # in exception and if we try to log it, it will fail.
+            try:
+                path = req.environ["PATH_INFO"]
+            except Exception:
+                path = "unknown"
+
+            LOG.error('Failed to parse request URL / path "%s": %s' % (path, str(e)))
+
+            abort(
+                400,
+                'Failed to parse request path "%s". URL likely contains invalid or incorrectly '
+                "URL encoded values." % (path),
+            )
+            return
+
         LOG.debug("Match path: %s", path)
 
         if len(path) > 1 and path.endswith("/"):