Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

BinaryVDF.test_loads_utf16 not passed #33

Open
tim77 opened this issue Jan 25, 2021 · 2 comments · May be fixed by #57
Open

BinaryVDF.test_loads_utf16 not passed #33

tim77 opened this issue Jan 25, 2021 · 2 comments · May be fixed by #57
Labels
bug

Comments

@tim77
Copy link

tim77 commented Jan 25, 2021

Some tests not passed in Fedora 32:

=================================== FAILURES ===================================
__________________________ BinaryVDF.test_loads_utf16 __________________________
self = <tests.test_binary_vdf.BinaryVDF testMethod=test_loads_utf16>
    def test_loads_utf16(self):
>       self.assertEqual({'aaa': b'b\x00b\x00\xff\xffb\x00b\x00'.decode('utf-16le')}, vdf.binary_loads(b'\x05aaa\x00b\x00b\x00\xff\xffb\x00b\x00\x00\x00\x08'))
E       AssertionError: {'aaa': 'bb\uffffbb'} != {'aaa': '戀戀\uffff戀戀'}
E       - {'aaa': 'bb\uffffbb'}
E       ?          ^^      ^^
E       
E       + {'aaa': '戀戀\uffff戀戀'}
E       ?          ^^      ^^
tests/test_binary_vdf.py:188: AssertionError

But tests passed in Fedora 33 and no such issue. Full F32 build log.

Additional information:

  • OS: Fedora 32
  • Python ver: 3.8.7
@rossengeorgiev
Copy link
Contributor

rossengeorgiev commented Jan 27, 2021 •
edited

That could only happen at https://github.com/ValvePython/vdf/blob/master/vdf/__init__.py#L361

Which means that utf-16 on that build host must have been understood as utf-16be (platform dependent), and since the test uses LE there will be a mismatch. Looking at https://kojipkgs.fedoraproject.org//work/tasks/713/60470713/hw_info.log confirms it.

I'd say this is a bug and I suppose the temporary fix is set the decode to utf-16le. I'm yet to see a single example of Valve using this type anywhere. It's all utf-8. Now that I think of it, I suppose all encode/decode should be LE, otherwise it all breaks on a BE system.

@tim77 tim77 mentioned this issue Aug 19, 2022
Closed
@smcv
Copy link
Contributor

smcv commented Dec 15, 2023

utf-16 on that build host must have been understood as utf-16be (platform dependent)

@tim77, if you're reporting a build/test failure on an unusual CPU architecture, please say so up-front? Otherwise maintainers will tend to assume that you were building on x86.

Debian s390x (which is big-endian) is exhibiting the same build failure (after I fixed a packaging bug that meant we weren't actually running the tests as we intended to). I'll send a merge request shortly.

smcv added a commit to smcv/vdf that referenced this issue Dec 15, 2023
@smcv
Treat UTF-16 strings in binary VDF as little-endian
fe85ab8 
Integers in binary VDF are already treated as little-endian (least
significant byte first) regardless of CPU architecture, but the 16-bit
units in UTF-16 didn't get the same treatment. This led to a test failure
on big-endian machines.

Resolves: ValvePython#33
Signed-off-by: Simon McVittie <smcv@debian.org>
@smcv smcv linked a pull request Dec 15, 2023 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Treat UTF-16 strings in binary VDF as little-endian smcv/vdf
3 participants
@smcv @rossengeorgiev @tim77