rusackas commented on code in PR #40860:
URL: https://github.com/apache/superset/pull/40860#discussion_r3408594821
##########
superset/commands/database/uploaders/base.py:
##########
@@ -188,6 +190,43 @@ def run(self) -> None:
sqla_table.fetch_metadata()
+ @staticmethod
+ def _file_size_bytes(file: Any) -> Optional[int]:
+ """
+ Return the size of an uploaded file without consuming its stream.
+
+ Returns ``None`` when the stream is not seekable, in which case the
+ size cannot be determined cheaply and the size check is skipped in
+ favour of downstream guards.
+ """
+ stream = getattr(file, "stream", file)
+ try:
+ position = stream.tell()
+ stream.seek(0, 2) # seek to end
+ size = stream.tell()
+ stream.seek(position) # restore the original position
+ except (AttributeError, OSError):
+ return None
+ return size
+
+ @classmethod
+ def validate_file_size(cls, file: Any) -> None:
+ """
+ Reject a file whose size exceeds ``UPLOAD_MAX_FILE_SIZE_BYTES``.
+
+ Shared by the upload command and the metadata endpoint so oversized
+ files are rejected before their contents are read into memory,
+ regardless of which path is used.
+
+ :raises DatabaseUploadFileTooLarge: if the file is larger than the
limit
+ """
+ max_file_size = current_app.config.get("UPLOAD_MAX_FILE_SIZE_BYTES")
+ if max_file_size is None or file is None:
+ return
+ size = cls._file_size_bytes(file)
+ if size is not None and size > max_file_size:
+ raise DatabaseUploadFileTooLarge()
Review Comment:
The 100 MB default is intentional, not a regression - it's the bound
introduced in #40637, and this PR just lifts the check up to
`UploadCommand.validate` so CSV/Excel and the metadata endpoint get it too. Set
`UPLOAD_MAX_FILE_SIZE_BYTES = None` to disable. Reconciled the summary and
added an UPDATING.md note.
##########
superset/databases/api.py:
##########
@@ -1718,6 +1718,15 @@ def upload_metadata(self) -> Response:
$ref: '#/components/responses/401'
404:
$ref: '#/components/responses/404'
+ 413:
+ description: Payload too large, file exceeds the maximum allowed
size
+ content:
+ application/json:
+ schema:
+ type: object
+ properties:
+ message:
+ type: string
Review Comment:
Matching the house style here on purpose: every FAB shared error response in
this file (400/401/422) documents `{message: string}` even though
`CommandException`s actually serialize as the `errors: [...]` envelope. So the
413 is consistent with its neighbors. Making just this one accurate would leave
it as the odd one out - if we want the real envelope shape it's a broader doc
cleanup, not this PR.
--
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]
