korbit-ai[bot] commented on code in PR #35219:
URL: https://github.com/apache/superset/pull/35219#discussion_r2364604727
##########
superset/dashboards/api.py:
##########
@@ -1342,6 +1342,8 @@ def thumbnail(self, pk: int, digest: str, **kwargs: Any)
-> WerkzeugResponse:
self.incr_stats("from_cache", self.thumbnail.__name__)
try:
image = cache_payload.get_image()
+ if not image or not hasattr(image, "read"):
+ return self.response_404()
Review Comment:
### Overly restrictive hasattr check for read attribute <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The defensive check uses `hasattr(image, "read")` which can return False for
valid file-like objects that have a `read` attribute but it's not accessible
due to various reasons (e.g., closed file, permission issues), potentially
causing false positives.
###### Why this matters
This could incorrectly return 404 for valid images that temporarily don't
have accessible read attributes, leading to unnecessary failures when the image
could be successfully processed.
###### Suggested change ∙ *Feature Preview*
Use a more robust check that attempts to verify the object is file-like by
checking if it's callable or use duck typing with a try-except block:
```python
if not image or not (hasattr(image, "read") and callable(getattr(image,
"read", None))):
return self.response_404()
```
Or alternatively:
```python
if not image:
return self.response_404()
try:
# Verify the read method exists and is callable
if not callable(getattr(image, "read", None)):
return self.response_404()
except AttributeError:
return self.response_404()
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/1aaa6640-d688-4d12-a1f0-6ff5d2272b28/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/1aaa6640-d688-4d12-a1f0-6ff5d2272b28?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/1aaa6640-d688-4d12-a1f0-6ff5d2272b28?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/1aaa6640-d688-4d12-a1f0-6ff5d2272b28?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/1aaa6640-d688-4d12-a1f0-6ff5d2272b28)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:ffa63663-843b-453c-8ec6-5461065ba3e5 -->
[](ffa63663-843b-453c-8ec6-5461065ba3e5)
--
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]
