Skip to content

Allow decoding files with invalid utf-8 - #21

Open
davisdude wants to merge 7 commits into
Iotic-Labs:dev-contribfrom
davisdude:invalid-utf8
Open

Allow decoding files with invalid utf-8#21
davisdude wants to merge 7 commits into
Iotic-Labs:dev-contribfrom
davisdude:invalid-utf8

Conversation

@davisdude

@davisdude davisdude commented Mar 22, 2025

Copy link
Copy Markdown

Summary

This was inspired by my work with a slippi replay file (attached). The replay file contained a string with invalid utf-8 data. I would like to be able to parse this replay file.

Changes

I updated both the C and Python code and added some new test cases for handling invalid string data. Note that I did not update the encode functionality.

Tests

All tests still pass (except the recursion test, which I disabled locally for testing):

$ ./coverage_test.sh

Removing _ubjson.cpython-313-x86_64-linux-gnu.so
Removing build/
running build_ext
building '_ubjson' extension
creating build/temp.linux-x86_64-cpython-313/src
gcc -fno-strict-overflow -Wsign-compare -DNDEBUG -g -O3 -Wall -march=x86-64 -mtune=generic -O3 -pipe -fno-plt -fexceptions -Wp,-D_FORTIFY_SOURCE=3 -Wformat -Werror=format-security -fstack-clash-protection -fcf-protection -fno-omit-frame-pointer -mno-omit-leaf-frame-pointer -g -ffile-prefix-map=/build/python/src=/usr/src/debug/python -flto=auto -ffat-lto-objects -march=x86-64 -mtune=generic -O3 -pipe -fno-plt -fexceptions -Wp,-D_FORTIFY_SOURCE=3 -Wformat -Werror=format-security -fstack-clash-protection -fcf-protection -fno-omit-frame-pointer -mno-omit-leaf-frame-pointer -g -ffile-prefix-map=/build/python/src=/usr/src/debug/python -flto=auto -march=x86-64 -mtune=generic -O3 -pipe -fno-plt -fexceptions -Wp,-D_FORTIFY_SOURCE=3 -Wformat -Werror=format-security -fstack-clash-protection -fcf-protection -fno-omit-frame-pointer -mno-omit-leaf-frame-pointer -g -ffile-prefix-map=/build/python/src=/usr/src/debug/python -flto=auto -coverage -fPIC -I/usr/include/python3.13 -c src/_ubjson.c -o build/temp.linux-x86_64-cpython-313/src/_ubjson.o -std=c99
gcc -fno-strict-overflow -Wsign-compare -DNDEBUG -g -O3 -Wall -march=x86-64 -mtune=generic -O3 -pipe -fno-plt -fexceptions -Wp,-D_FORTIFY_SOURCE=3 -Wformat -Werror=format-security -fstack-clash-protection -fcf-protection -fno-omit-frame-pointer -mno-omit-leaf-frame-pointer -g -ffile-prefix-map=/build/python/src=/usr/src/debug/python -flto=auto -ffat-lto-objects -march=x86-64 -mtune=generic -O3 -pipe -fno-plt -fexceptions -Wp,-D_FORTIFY_SOURCE=3 -Wformat -Werror=format-security -fstack-clash-protection -fcf-protection -fno-omit-frame-pointer -mno-omit-leaf-frame-pointer -g -ffile-prefix-map=/build/python/src=/usr/src/debug/python -flto=auto -march=x86-64 -mtune=generic -O3 -pipe -fno-plt -fexceptions -Wp,-D_FORTIFY_SOURCE=3 -Wformat -Werror=format-security -fstack-clash-protection -fcf-protection -fno-omit-frame-pointer -mno-omit-leaf-frame-pointer -g -ffile-prefix-map=/build/python/src=/usr/src/debug/python -flto=auto -coverage -fPIC -I/usr/include/python3.13 -c src/decoder.c -o build/temp.linux-x86_64-cpython-313/src/decoder.o -std=c99
gcc -fno-strict-overflow -Wsign-compare -DNDEBUG -g -O3 -Wall -march=x86-64 -mtune=generic -O3 -pipe -fno-plt -fexceptions -Wp,-D_FORTIFY_SOURCE=3 -Wformat -Werror=format-security -fstack-clash-protection -fcf-protection -fno-omit-frame-pointer -mno-omit-leaf-frame-pointer -g -ffile-prefix-map=/build/python/src=/usr/src/debug/python -flto=auto -ffat-lto-objects -march=x86-64 -mtune=generic -O3 -pipe -fno-plt -fexceptions -Wp,-D_FORTIFY_SOURCE=3 -Wformat -Werror=format-security -fstack-clash-protection -fcf-protection -fno-omit-frame-pointer -mno-omit-leaf-frame-pointer -g -ffile-prefix-map=/build/python/src=/usr/src/debug/python -flto=auto -march=x86-64 -mtune=generic -O3 -pipe -fno-plt -fexceptions -Wp,-D_FORTIFY_SOURCE=3 -Wformat -Werror=format-security -fstack-clash-protection -fcf-protection -fno-omit-frame-pointer -mno-omit-leaf-frame-pointer -g -ffile-prefix-map=/build/python/src=/usr/src/debug/python -flto=auto -coverage -fPIC -I/usr/include/python3.13 -c src/encoder.c -o build/temp.linux-x86_64-cpython-313/src/encoder.o -std=c99
gcc -fno-strict-overflow -Wsign-compare -DNDEBUG -g -O3 -Wall -march=x86-64 -mtune=generic -O3 -pipe -fno-plt -fexceptions -Wp,-D_FORTIFY_SOURCE=3 -Wformat -Werror=format-security -fstack-clash-protection -fcf-protection -fno-omit-frame-pointer -mno-omit-leaf-frame-pointer -g -ffile-prefix-map=/build/python/src=/usr/src/debug/python -flto=auto -ffat-lto-objects -march=x86-64 -mtune=generic -O3 -pipe -fno-plt -fexceptions -Wp,-D_FORTIFY_SOURCE=3 -Wformat -Werror=format-security -fstack-clash-protection -fcf-protection -fno-omit-frame-pointer -mno-omit-leaf-frame-pointer -g -ffile-prefix-map=/build/python/src=/usr/src/debug/python -flto=auto -march=x86-64 -mtune=generic -O3 -pipe -fno-plt -fexceptions -Wp,-D_FORTIFY_SOURCE=3 -Wformat -Werror=format-security -fstack-clash-protection -fcf-protection -fno-omit-frame-pointer -mno-omit-leaf-frame-pointer -g -ffile-prefix-map=/build/python/src=/usr/src/debug/python -flto=auto -coverage -fPIC -I/usr/include/python3.13 -c src/python_funcs.c -o build/temp.linux-x86_64-cpython-313/src/python_funcs.o -std=c99
gcc -shared -Wl,-O1 -Wl,--sort-common -Wl,--as-needed -Wl,-z,relro -Wl,-z,now -Wl,-z,pack-relative-relocs -flto=auto -Wl,-O1 -Wl,--sort-common -Wl,--as-needed -Wl,-z,relro -Wl,-z,now -Wl,-z,pack-relative-relocs -flto=auto -coverage build/temp.linux-x86_64-cpython-313/src/_ubjson.o build/temp.linux-x86_64-cpython-313/src/decoder.o build/temp.linux-x86_64-cpython-313/src/encoder.o build/temp.linux-x86_64-cpython-313/src/python_funcs.o -L/usr/lib -o /home/davis/files/programming/python/py-ubjson/_ubjson.cpython-313-x86_64-linux-gnu.so
test_array (test.TestEncodeDecodeFp.test_array) ... ok
test_array_fixed (test.TestEncodeDecodeFp.test_array_fixed) ... ok
test_array_noop (test.TestEncodeDecodeFp.test_array_noop) ... ok
test_bool (test.TestEncodeDecodeFp.test_bool) ... ok
test_bytes (test.TestEncodeDecodeFp.test_bytes) ... ok
test_char (test.TestEncodeDecodeFp.test_char) ... ok
test_circular (test.TestEncodeDecodeFp.test_circular) ... ok
test_decode_exception_position (test.TestEncodeDecodeFp.test_decode_exception_position) ... ok
test_decode_object_hook (test.TestEncodeDecodeFp.test_decode_object_hook) ... ok
test_decoder_fuzz (test.TestEncodeDecodeFp.test_decoder_fuzz) ... ok
test_encode_default (test.TestEncodeDecodeFp.test_encode_default) ... ok
test_float (test.TestEncodeDecodeFp.test_float) ... ok
test_fp (test.TestEncodeDecodeFp.test_fp) ... ok
test_high_precision (test.TestEncodeDecodeFp.test_high_precision) ... ok
test_int (test.TestEncodeDecodeFp.test_int) ... ok
test_intern_object_keys (test.TestEncodeDecodeFp.test_intern_object_keys) ... ok
test_invalid_data (test.TestEncodeDecodeFp.test_invalid_data) ... ok
test_invalid_fp_dump (test.TestEncodeDecodeFp.test_invalid_fp_dump) ... ok
test_invalid_fp_load (test.TestEncodeDecodeFp.test_invalid_fp_load) ... ok
test_invalid_marker (test.TestEncodeDecodeFp.test_invalid_marker) ... ok
test_no_data (test.TestEncodeDecodeFp.test_no_data) ... ok
test_null (test.TestEncodeDecodeFp.test_null) ... ok
test_object (test.TestEncodeDecodeFp.test_object) ... ok
test_object_fixed (test.TestEncodeDecodeFp.test_object_fixed) ... ok
test_object_invalid (test.TestEncodeDecodeFp.test_object_invalid) ... ok
test_object_noop (test.TestEncodeDecodeFp.test_object_noop) ... ok
test_string (test.TestEncodeDecodeFp.test_string) ... ok
test_trailing_input (test.TestEncodeDecodeFp.test_trailing_input) ... ok
test_unencodable (test.TestEncodeDecodeFp.test_unencodable) ... ok
test_array (test.TestEncodeDecodeFpExt.test_array) ... ok
test_array_fixed (test.TestEncodeDecodeFpExt.test_array_fixed) ... ok
test_array_noop (test.TestEncodeDecodeFpExt.test_array_noop) ... ok
test_bool (test.TestEncodeDecodeFpExt.test_bool) ... ok
test_bytes (test.TestEncodeDecodeFpExt.test_bytes) ... ok
test_char (test.TestEncodeDecodeFpExt.test_char) ... ok
test_circular (test.TestEncodeDecodeFpExt.test_circular) ... ok
test_decode_exception_position (test.TestEncodeDecodeFpExt.test_decode_exception_position) ... ok
test_decode_object_hook (test.TestEncodeDecodeFpExt.test_decode_object_hook) ... ok
test_decoder_fuzz (test.TestEncodeDecodeFpExt.test_decoder_fuzz) ... ok
test_encode_default (test.TestEncodeDecodeFpExt.test_encode_default) ... ok
test_float (test.TestEncodeDecodeFpExt.test_float) ... ok
test_fp (test.TestEncodeDecodeFpExt.test_fp) ... ok
test_fp_buffer (test.TestEncodeDecodeFpExt.test_fp_buffer) ... ok
test_fp_callable_incomplete (test.TestEncodeDecodeFpExt.test_fp_callable_incomplete) ... ok
test_fp_multi (test.TestEncodeDecodeFpExt.test_fp_multi) ... ok
test_fp_seek_invalid (test.TestEncodeDecodeFpExt.test_fp_seek_invalid) ... ok
test_high_precision (test.TestEncodeDecodeFpExt.test_high_precision) ... ok
test_int (test.TestEncodeDecodeFpExt.test_int) ... ok
test_intern_object_keys (test.TestEncodeDecodeFpExt.test_intern_object_keys) ... ok
test_invalid_data (test.TestEncodeDecodeFpExt.test_invalid_data) ... ok
test_invalid_fp_dump (test.TestEncodeDecodeFpExt.test_invalid_fp_dump) ... ok
test_invalid_fp_load (test.TestEncodeDecodeFpExt.test_invalid_fp_load) ... ok
test_invalid_marker (test.TestEncodeDecodeFpExt.test_invalid_marker) ... ok
test_no_data (test.TestEncodeDecodeFpExt.test_no_data) ... ok
test_null (test.TestEncodeDecodeFpExt.test_null) ... ok
test_object (test.TestEncodeDecodeFpExt.test_object) ... ok
test_object_fixed (test.TestEncodeDecodeFpExt.test_object_fixed) ... ok
test_object_invalid (test.TestEncodeDecodeFpExt.test_object_invalid) ... ok
test_object_noop (test.TestEncodeDecodeFpExt.test_object_noop) ... ok
test_string (test.TestEncodeDecodeFpExt.test_string) ... ok
test_trailing_input (test.TestEncodeDecodeFpExt.test_trailing_input) ... ok
test_unencodable (test.TestEncodeDecodeFpExt.test_unencodable) ... ok
test_array (test.TestEncodeDecodePlain.test_array) ... ok
test_array_fixed (test.TestEncodeDecodePlain.test_array_fixed) ... ok
test_array_noop (test.TestEncodeDecodePlain.test_array_noop) ... ok
test_bool (test.TestEncodeDecodePlain.test_bool) ... ok
test_bytes (test.TestEncodeDecodePlain.test_bytes) ... ok
test_char (test.TestEncodeDecodePlain.test_char) ... ok
test_circular (test.TestEncodeDecodePlain.test_circular) ... ok
test_decode_object_hook (test.TestEncodeDecodePlain.test_decode_object_hook) ... ok
test_decoder_fuzz (test.TestEncodeDecodePlain.test_decoder_fuzz) ... ok
test_encode_default (test.TestEncodeDecodePlain.test_encode_default) ... ok
test_float (test.TestEncodeDecodePlain.test_float) ... ok
test_high_precision (test.TestEncodeDecodePlain.test_high_precision) ... ok
test_int (test.TestEncodeDecodePlain.test_int) ... ok
test_intern_object_keys (test.TestEncodeDecodePlain.test_intern_object_keys) ... ok
test_invalid_data (test.TestEncodeDecodePlain.test_invalid_data) ... ok
test_invalid_marker (test.TestEncodeDecodePlain.test_invalid_marker) ... ok
test_no_data (test.TestEncodeDecodePlain.test_no_data) ... ok
test_null (test.TestEncodeDecodePlain.test_null) ... ok
test_object (test.TestEncodeDecodePlain.test_object) ... ok
test_object_fixed (test.TestEncodeDecodePlain.test_object_fixed) ... ok
test_object_invalid (test.TestEncodeDecodePlain.test_object_invalid) ... ok
test_object_noop (test.TestEncodeDecodePlain.test_object_noop) ... ok
test_string (test.TestEncodeDecodePlain.test_string) ... ok
test_trailing_input (test.TestEncodeDecodePlain.test_trailing_input) ... ok
test_unencodable (test.TestEncodeDecodePlain.test_unencodable) ... ok
test_array (test.TestEncodeDecodePlainExt.test_array) ... ok
test_array_fixed (test.TestEncodeDecodePlainExt.test_array_fixed) ... ok
test_array_noop (test.TestEncodeDecodePlainExt.test_array_noop) ... ok
test_bool (test.TestEncodeDecodePlainExt.test_bool) ... ok
test_bytes (test.TestEncodeDecodePlainExt.test_bytes) ... ok
test_char (test.TestEncodeDecodePlainExt.test_char) ... ok
test_circular (test.TestEncodeDecodePlainExt.test_circular) ... ok
test_decode_object_hook (test.TestEncodeDecodePlainExt.test_decode_object_hook) ... ok
test_decoder_fuzz (test.TestEncodeDecodePlainExt.test_decoder_fuzz) ... ok
test_encode_default (test.TestEncodeDecodePlainExt.test_encode_default) ... ok
test_float (test.TestEncodeDecodePlainExt.test_float) ... ok
test_high_precision (test.TestEncodeDecodePlainExt.test_high_precision) ... ok
test_int (test.TestEncodeDecodePlainExt.test_int) ... ok
test_intern_object_keys (test.TestEncodeDecodePlainExt.test_intern_object_keys) ... ok
test_invalid_data (test.TestEncodeDecodePlainExt.test_invalid_data) ... ok
test_invalid_marker (test.TestEncodeDecodePlainExt.test_invalid_marker) ... ok
test_no_data (test.TestEncodeDecodePlainExt.test_no_data) ... ok
test_null (test.TestEncodeDecodePlainExt.test_null) ... ok
test_object (test.TestEncodeDecodePlainExt.test_object) ... ok
test_object_fixed (test.TestEncodeDecodePlainExt.test_object_fixed) ... ok
test_object_invalid (test.TestEncodeDecodePlainExt.test_object_invalid) ... ok
test_object_noop (test.TestEncodeDecodePlainExt.test_object_noop) ... ok
test_string (test.TestEncodeDecodePlainExt.test_string) ... ok
test_trailing_input (test.TestEncodeDecodePlainExt.test_trailing_input) ... ok
test_unencodable (test.TestEncodeDecodePlainExt.test_unencodable) ... ok

----------------------------------------------------------------------
Ran 112 tests in 7.767s

OK
Wrote HTML report to coverage/python/index.html
Capturing coverage data from .
geninfo cmd: '/usr/bin/geninfo . --toolname lcov --output-filename /tmp/ubjson-coverage.info.pre'
Found gcov version: 14.2.1
Using intermediate gcov format
Recording 'internal' directories:
	/home/davis/files/programming/python/py-ubjson
	.
Writing temporary data to /tmp/geninfo_dat2HzG
Scanning . for .gcda files ...
Found 4 data files in .
using: chunkSize: 1, nchunks:4, intervalLength:0
lcov: WARNING: using JSON module "JSON::PP" - which is much slower than some alternatives.  Consider installing one of JSON::XS or Cpanel::JSON::XS
lcov: WARNING: (inconsistent) /usr/include/python3.13/object.h:949: unexecuted block on non-branch line with non-zero hit count.  Use "geninfo --rc geninfo_unexecuted_blocks=1 to set count to zero.
	(use "lcov --ignore-errors inconsistent,inconsistent ..." to suppress this warning)
Finished processing 4 GCDA files
Apply filtering..
Finished filter file processing
Finished .info-file creation
Summary coverage rate:
  source files: 10
  lines.......: 80.6% (994 of 1234 lines)
  functions...: 100.0% (54 of 54 functions)
Message summary:
  1 warning message:
    inconsistent: 1
Excluding /usr/include/bits/string_fortified.h
Excluding /usr/include/python3.13/cpython/bytearrayobject.h
Excluding /usr/include/python3.13/cpython/bytesobject.h
Excluding /usr/include/python3.13/cpython/listobject.h
Excluding /usr/include/python3.13/cpython/tupleobject.h
Excluding /usr/include/python3.13/object.h
Removed 6 files
Writing data to /tmp/ubjson-coverage.info.pre2
Summary coverage rate:
  source files: 4
  lines.......: 79.6% (934 of 1174 lines)
  functions...: 100.0% (53 of 53 functions)
Message summary:
  no messages were reported
Excluding /home/davis/files/programming/python/py-ubjson/src/python_funcs.c
Removed 1 files
Writing data to /tmp/ubjson-coverage.info
Summary coverage rate:
  source files: 3
  lines.......: 93.5% (882 of 943 lines)
  functions...: 100.0% (48 of 48 functions)
Message summary:
  no messages were reported
Reading tracefile /tmp/ubjson-coverage.info.
Found 3 entries.
Found common filename prefix "/home/davis/files/programming/python/py-ubjson"
Generating output.
Processing file src/decoder.c
  lines=513 hit=488 functions=24 hit=24
Processing file src/_ubjson.c
  lines=107 hit=98 functions=6 hit=6
Processing file src/encoder.c
  lines=323 hit=296 functions=18 hit=18
Overall coverage rate:
  source files: 3
  lines.......: 93.5% (882 of 943 lines)
  functions...: 100.0% (48 of 48 functions)
Message summary:
  no messages were reported

For coverage results see index.html in coverage sub-directories.

Additionally, the file that I couldn't parse before is now parsable!

>>> import ubjson
>>> with open("test.slp", "rb") as f:
...     ubjson.load(f)
...     
Traceback (most recent call last):
  File "<python-input-14>", line 2, in <module>
    ubjson.load(f)
    ~~~~~~~~~~~^^^
ubjson.decoder.DecoderException: ('Failed to decode utf8: string (at byte 2655127)', 2655127)
>>> 
>>> with open("test.slp", "rb") as f:
...     ubjson.load(f, errors='replace')
# ...
# 'consoleNick': 'Frankhuize\n\ufff7\uffff뽯\uffff���\uffff�', 'players': {}, 'playedOn': 'nintendont'}}
# ^ The problematic data

Attachments

  • coverage.zip: Code coverage results
  • test_slp.zip: The problematic slippi file, which wasn't being parsed properly before

@vtermanis

Copy link
Copy Markdown
Contributor

Hi @davisdude,

Thank you for your contribution! It'd definitely be useful to allow utf8 decoding error handling to be customised. From an initial look at your PR - you've considered everything, including tests, and updated the code sympathetic to the existing style - I appreciate that. 👍
However, there are few issues:

  1. Unfortunately I don't currently have write access to this repo so I cannot approve/merge/release etc. (When this will be possible again unfortunately I don't know - it could take a couple of months. I cannot say more than that since it regards the position of Iotic Labs.)

    • If nothing happens regarding this, maybe it'd be good to make a fork of it and enhance it as you see fit.
  2. With this extra option, I think there now are too many variables passed around for state (which some of the decode functions don't even need). I think it would be beneficial to store them in e.g. a named tuple and only pass that around. (The decode could of course also be a full class but maybe that is overkill.)

    • This would be something to consider in a separate PR and then build this feature on top. (It wouldn't affect the C-extension side since that already has the "conf" struct).
    • If I had write access, I'd be happy to implement this.
  3. I did not update the encode functionality.

    That's OK. However, the load function should document what errors does and the changelog should have a new entry to explain what has been added.

  4. It would be great to switch to e.g. pyproject (as per Transition to pyproject. #20) incorporate Fix recursion test case for Python 3.12 #19 and also migrate to using GH actions to deploy.

    • Otherwise it'd be a completely manual release. (Also - I think it woud be nice to have binary wheels for popular platforms available in PyPI and not necessarily bother with conda.)

I'm sorry this doesn't provide the result you're looking for right now. (I haven't had the time to put care into this repo over the last years.)

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.

2 participants