Re: [PR] fix: handle undefined template variables safely in query rendering. [superset]

["betodealmeida (via GitHub)" <gi...@apache.org>]

betodealmeida commented on code in PR #35009:
URL: https://github.com/apache/superset/pull/35009#discussion_r2518886696


##########
superset/jinja_context.py:
##########
@@ -696,6 +703,8 @@ def process_template(self, sql: str, **kwargs: Any) -> str:
         >>> process_template(sql)
         "SELECT '2017-01-01T00:00:00'"
         """
+        from jinja2 import UndefinedError

Review Comment:
   Let's move this to the top of the file, per convention.



##########
superset/sqllab/query_render.py:
##########
@@ -66,6 +66,19 @@ def render(self, execution_context: SqlJsonExecutionContext) -> str:
         except TemplateError as ex:
             self._raise_template_exception(ex, execution_context)
             return "NOT_REACHABLE_CODE"
+        except Exception as ex:
+            from superset.jinja_context import UndefinedTemplateFunctionException
+
+            if isinstance(ex, UndefinedTemplateFunctionException):
+                return query_model.sql.strip().strip(";")
+            raise
+
+    def _strip_sql_comments(self, sql: str) -> str:
+        import re
+
+        sql = re.sub(r"/\*.*?\*/", "", sql, flags=re.DOTALL)
+        sql = re.sub(r"--[^\n\r]*", "", sql)
+        return sql

Review Comment:
   The regex approach is flaky, it can't handle nested comments (supported by some dialects) and might produce invalid SQL:
   
   ```python
   >>> sql = "SELECT '--debug' AS flag"
   >>> re.sub(r"--[^\n\r]*", "", sql)
   "SELECT '"
   ```
   
   Since you have a reference to the database it's better to use the SQL parser:
   
   ```python
   dialect = execution_context.query.database.db_engine.spec.engine
   sql_for_validation = SQLScript(sql, dialect).format(comments=False)
   ```
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org