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
@KazuCocoa KazuCocoa added the size:XS contribution size: XS label Oct 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XS contribution size: XS

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants