Skip to content

Fix serialization of invalid chars in C#612

Merged
chvp merged 2 commits intomasterfrom
fix/c-invalid-char-serialization
Feb 9, 2026
Merged

Fix serialization of invalid chars in C#612
chvp merged 2 commits intomasterfrom
fix/c-invalid-char-serialization

Conversation

@chvp
Copy link
Member

@chvp chvp commented Feb 6, 2026

Also took the opportunity to format the serialization a bit better.

Invalid characters are now treated as unsigned chars and written out in hex format (e.g. 0xA4). Better suggestions are welcome.

@chvp chvp requested a review from niknetniko February 6, 2026 10:17
@chvp chvp self-assigned this Feb 6, 2026
@chvp chvp added the bug Something isn't working label Feb 6, 2026
@chvp chvp force-pushed the fix/c-invalid-char-serialization branch from eb93056 to b67b662 Compare February 6, 2026 10:19
@bmesuere
Copy link
Member

bmesuere commented Feb 6, 2026

I think the C++ implementation needs a similar fix?

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This pull request addresses serialization issues with invalid characters in C by implementing hex formatting for characters with the high bit set. The PR also improves code formatting in the escape function and adds a test for detecting wrong function signatures in C.

Changes:

  • Modified the escape() function to output high-bit characters in hex format (e.g., 0xA4) instead of attempting standard escaping
  • Improved code formatting and structure in the escape function
  • Added test case for detecting C function signature mismatches (returning wrong type)

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
tested/languages/c/templates/values.c Updated escape function to handle high-bit characters with hex formatting; improved formatting and memory allocation
tests/test_language_quirks.py Added test for detecting wrong function return type in C
tests/exercises/echo-function/solution/wrong-signature.c Added test solution file with intentional type mismatch (returns char instead of char*)
Comments suppressed due to low confidence (2)

tests/test_language_quirks.py:29

  • The test name test_c_pointer_char_return doesn't clearly describe what's being tested. Consider renaming to something more descriptive like test_c_wrong_return_type_signature or test_c_return_type_mismatch_detection to better convey that this test verifies the system correctly detects when a function returns the wrong type (char instead of char*).
    tests/test_language_quirks.py:40
  • The new test test_c_pointer_char_return is testing for a wrong function signature (returning char instead of char*), but it doesn't actually test the new hex formatting functionality for invalid characters that was added in this PR. Consider adding a test case that passes a string containing high-bit or non-printable characters to verify that the hex formatting (e.g., 0xA4) works correctly.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@chvp
Copy link
Member Author

chvp commented Feb 6, 2026

At least not the same fix; the serialization uses std::string, not sure how that behaves. https://github.com/dodona-edu/universal-judge/blob/master/tested/languages/cpp/templates/values.tpp#L66

@chvp chvp force-pushed the fix/c-invalid-char-serialization branch from b67b662 to 641d2d6 Compare February 6, 2026 13:25
@chvp
Copy link
Member Author

chvp commented Feb 6, 2026

Did some tests, this is much harder to trigger in cpp, but still possible, so I added a fix for that serializer as well.

@chvp chvp force-pushed the fix/c-invalid-char-serialization branch from 5ab360d to 8440326 Compare February 8, 2026 15:56
@chvp chvp force-pushed the fix/c-invalid-char-serialization branch from 8440326 to 959a2b9 Compare February 8, 2026 16:00
Copy link
Member

@niknetniko niknetniko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems a reasonable solution to me.

@chvp chvp merged commit 72d62b7 into master Feb 9, 2026
9 checks passed
@chvp chvp deleted the fix/c-invalid-char-serialization branch February 9, 2026 09:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants