Skip to content

fix: prevent IndexError in Type action parser and fix screenshot temp file leak#394

Open
Ricardo-M-L wants to merge 2 commits intozai-org:mainfrom
Ricardo-M-L:fix/action-parser-and-screenshot-cleanup
Open

fix: prevent IndexError in Type action parser and fix screenshot temp file leak#394
Ricardo-M-L wants to merge 2 commits intozai-org:mainfrom
Ricardo-M-L:fix/action-parser-and-screenshot-cleanup

Conversation

@Ricardo-M-L
Copy link
Copy Markdown

Summary

Two bug fixes:

1. IndexError in parse_action() for Type actions (handler.py:351)

response.split("text=", 1)[1] assumes the response always contains text=. A malformed Type action like do(action="Type") without a text parameter causes IndexError: list index out of range.

Fix: Check split result length before accessing [1], raise clear ValueError.

2. Temp file leak in ADB screenshot capture (screenshot.py:65-85)

Temp files created with UUID names are only cleaned up on the success path (os.remove at line 77). If Image.open(), img.save(), or base64.b64encode() raises an exception, the temp file leaks in /tmp.

Fix: Wrap image processing in try/finally to ensure cleanup regardless of exceptions.

Ricardo-M-L and others added 2 commits April 14, 2026 22:12
… file leak

1. handler.py parse_action(): split("text=", 1) can produce a single-
   element list when the text parameter is missing from a malformed Type
   action. Add a length check before accessing [1] and raise a clear
   ValueError instead of an opaque IndexError.

2. adb/screenshot.py: temp files created with unique UUIDs are only
   cleaned up on the success path. If Image.open() or img.save() raises,
   the file leaks. Wrap the image processing in try/finally and also
   clean up in the outer except block.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The HarmonyOS HDC path in phone_agent/hdc/screenshot.py had the
identical leak pattern as the ADB path — if Image.open() or
img.save() raised, the outer except swallowed the error and the
temp file under $TMPDIR was never removed.

Apply the same try/finally + except-path cleanup as ADB to guarantee
the temp screenshot file is removed on every path.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@Ricardo-M-L
Copy link
Copy Markdown
Author

Pushed one more commit (ac3c797) applying the same leak fix to phone_agent/hdc/screenshot.py (the HarmonyOS sibling of the ADB screenshot module). It has the identical try-without-finally pattern — if Image.open() or img.save() raised, the temp screenshot under $TMPDIR was never removed.

The fix mirrors the ADB one exactly:

-        img = Image.open(temp_path)
-        ...
-        os.remove(temp_path)
-        return Screenshot(...)
-    except Exception as e:
-        print(f"Screenshot error: {e}")
-        return _create_fallback_screenshot(...)
+        try:
+            img = Image.open(temp_path)
+            ...
+            return Screenshot(...)
+        finally:
+            if os.path.exists(temp_path):
+                os.remove(temp_path)
+    except Exception as e:
+        print(f"Screenshot error: {e}")
+        if os.path.exists(temp_path):
+            os.remove(temp_path)
+        return _create_fallback_screenshot(...)

Happy to split this into a separate PR if you'd prefer — it keeps the scope narrow ("temp file leak" in both OS paths), so bundling felt natural, but let me know.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant