Skip to content

Conversation

wooyoung294
Copy link
Contributor

Relates to #1169

Thanks for the fix! #1170

I opened a PR to normalize str|bytes to a base64 string so the JSON payload is always valid.

The following test passes on the current implementation

def _adjust_image_payload(payload: Union[str,bytes]) -> str:
    return payload if isinstance(payload, str) else payload.decode('utf-8')

def test_non_utf8_payload_bytes_raise_unicode_decode_error():
    payload = b"\x89PNG\r\n\x1a\n\x00\x00\x00IHDR"
    with pytest.raises(UnicodeDecodeError):
        _adjust_image_payload(payload)

Copy link

linux-foundation-easycla bot commented Sep 10, 2025

CLA Signed

The committers listed above are authorized under a signed CLA.

@KazuCocoa
Copy link
Member

Could you share an example of the incoming JSON that had \x89PNG\r\n\x1a\n\x00\x00\x00IHDR as your attached case?

@KazuCocoa KazuCocoa changed the title refact: _adjust_image_payload refactor: _adjust_image_payload Sep 10, 2025
@wooyoung294
Copy link
Contributor Author

Could you share an example of the incoming JSON that had \x89PNG\r\n\x1a\n\x00\x00\x00IHDR as your attached case?

@KazuCocoa @mykola-mokhnach Thanks for the review!

When you read a PNG in "rb" mode you get raw bytes like

b'\x89PNG\r\n\x1a\n\x00\x00\x00\rIHDR\x00\...

so "_adjust_image_payload" raises UnicodeDecodeError

def _adjust_image_payload(payload: Union[str,bytes]) -> str:
    return payload if isinstance(payload, str) else payload.decode('utf-8')

def test_non_utf8_payload_bytes_raise_unicode_decode_error():
    # i used https://png.pngtree.com/png-vector/20190411/ourmid/pngtree-vector-business-men-icon-png-image_925963.jpg
    path = "your.png"

    png_img = open(path, "rb").read()

    print(png_img,"\n") # b'\x89PNG\r\n\x1a\n\x00\x00\x00\rIHDR\x00\....
    print("path_type:",type(png_img),"\n") # <class 'bytes'>
    print(isinstance(png_img, bytes),"\n") # True

    _adjust_image_payload(png_img)

And I guess the line

data = utils.dump_json(param) in "def excute" does JSON serialization, so i modified it this way

def execute(self, command, params):
        """Send a command to the remote server.

        Any path substitutions required for the URL mapped to the command should be
        included in the command parameters.

        :Args:
         - command - A string specifying the command to execute.
         - params - A dictionary of named parameters to send with the command as
           its JSON payload.
        """
        command_info = self._commands.get(command) or self.extra_commands.get(command)
        ...
        data = utils.dump_json(params)

Thank you for taking the time to read through this long note.
I really appreciate all your work on appium-python-client, which I use a lot.
If this PR is off track or not appropriate, please feel free to close it.

@KazuCocoa
Copy link
Member

Got it, thanks. We expect to get a base64-encoded string as the input. For example, match_images_features expects:

        Args:
            base64_image1: base64-encoded content of the first image
            base64_image2: base64-encoded content of the second image

e.g.
https://github.com/wooyoung294/python-client/blob/5f153e39caa5dba934ff39879f5c18f3ff248656/appium/webdriver/extensions/images_comparison.py#L37C1-L39C70

https://appium.github.io/python-client-sphinx/webdriver.extensions.html#webdriver.extensions.images_comparison.ImagesComparison.match_images_features

@mykola-mokhnach
Copy link
Contributor

mykola-mokhnach commented Sep 11, 2025

What we may do is to show a customized error message if UnicodeDecodeError pops up to help users figure out the issue, e.g.

try:
    return payload if isinstance(payload, str) else payload.decode('utf-8')
except UnicodeDecodeError as e:
    raise ValueError('The image payload cannot be serialized to a string. Make sure to base64-encode it first') from e

@wooyoung294
Copy link
Contributor Author

wooyoung294 commented Sep 11, 2025

Thanks for guide and feedback!!

Applied the suggestion—now raising a helpful ValueError on UnicodeDecodeError

@mykola-mokhnach
Copy link
Contributor

https://github.com/appium/python-client/actions/runs/17635493188/job/50111937540?pr=1172

Please apply formatting to pass unit tests (make format)

@KazuCocoa KazuCocoa merged commit dc301ec into appium:master Sep 11, 2025
9 of 11 checks passed
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.

3 participants