codeant-ai-for-open-source[bot] commented on PR #36792: URL: https://github.com/apache/superset/pull/36792#issuecomment-3681704376
## Nitpicks 🔍 <table> <tr><td>🔒 <strong>No security issues identified</strong></td></tr> <tr><td>⚡ <strong>Recommended areas for review</strong><br><br> - [ ] <a href='https://github.com/apache/superset/pull/36792/files#diff-211594e63064e31f46f2682b01e1b44e8aa9a354bebbe0dc2d43888c91f63397R157-R169'><strong>Validation ordering</strong></a><br>The new schema-normalization (treating empty/"undefined" as None) happens after calling `self.validate()`. Because `validate()` calls `schema_allows_file_upload(self._model, self._schema)` using the original `self._schema`, requests where the frontend sends empty/"undefined" may be rejected before the normalization occurs. Consider normalizing the schema before validation so the permission check and subsequent logic use the intended schema value.<br> - [ ] <a href='https://github.com/apache/superset/pull/36792/files#diff-211594e63064e31f46f2682b01e1b44e8aa9a354bebbe0dc2d43888c91f63397R162-R168'><strong>Schema normalization robustness</strong></a><br>The added check only compares literally to the string "undefined" and treats falsy values as empty. It doesn't handle whitespace-only strings, different casing (e.g., "Undefined"), or other common tokens like "null". This can lead to unexpected behavior if the frontend sends one of those variants. Normalize the schema string (strip + lower) and check a small set of sentinel values.<br> </td></tr> </table> -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
