Open grossws opened 7 years ago
As I found bug with incorrect handling of null-terminated multibyte strings (NTMBS) was introduced in quite old commit.
That commit introduced two things:
null
s in stream when decoding ffi call result in StringResultConverter#fromNative
depending on defaultCharset
(aka java file.encoding
parameter or external encoding).Second part is fragile for both NTMBS and null-terminated widechar strings (NTWS) which can lead to several issues.
wchar_t *
) partially on UTF-8 system. E. g. on Linux wide string L"test"
returned from C would result 74 00 00 00
for first wchar_t
in .rodata
section on little-endian CPU and only t
will be read by jnr-ffi because terminatorWidth
is 1 for UTF-8
.Since UTF-8
is common across Linux and MacOSX (and for lot of other systems, I guess) for OpenJDK and OracleJDK that commit accidentaly works fine. And it works for explicitly mentioned US-ASCII
and ISO 8859-1
single-byte encodins because terminatorWidth
is also set to 1
for them.
Also, passing string-like to wchar_t *
arguments seems to be broken too. If I pass java string "ab"
to funtion void wstring_dump(const wchar_t *)
first wchar_t
is 61 62 00 00
intead on 61 00 00 00
.
So, it's totally broken for wchar_t *
strings which support was declared goal of mentioned commit and is broken for almost all encodings when used with char *
.
I think, proper fix requires to split handing of char *
(NTMBS) and wchar_t *
(NTWS) strings, add annotation to allow user say that NTWS is passed/returned and revert NTMBS handling code to use single null char
as NTMBS. Passing strings with 4 nulls is safe and should be continued since it prevents C side from reading past end of string if char *
was passed instead of wchar_t *
.
Hello there!
This is a good find. I'm not surprised to find some problems dealing with non-UTF-8 encodings when passing strings in and out of jnr-ffi. I also agree with your assessment; there need to be a way to specify how strings are expected to be encoded when coming from a library.
Do you perhaps have some idea of the change to make? You've managed to find the problem and diagnose it pretty well...perhaps you can put together a PR that starts making things better?
@headius, I'm not yet familar with jnr-ffi
codebase and lack of spare time now to fix this issue. IIRC, jna
had separation for string
/wstring
types to avoid such issues.
I'll have this issue in mind if I have several days to dig into but can't promise anything. Also I have no systems with Windows running and this issue doesn't affect me, so it has quite low priority in my list. If someone steps up to fix it, I'll be happy with it.
This bug bit me today. I don't know how to do a general fix, but for this specific instance you can use the @Encoding
annotation to specify that the method returns a US-ASCII
encoded string. See https://github.com/abstractj/kalium/issues/87 for details.
When JVM is started with single-byte default charset string read from native lib are handled incorrectly which results to reading
text
section (in case of static strings in binary) untill next doubleNUL
-byte is met.Bug found during trying to show issues with implicit use of
defaultEncoding
inString#getBytes()
in kalium to make PR with forcing use ofstr.getBytes(StandardCharset.UTF_8)
or something similar. And it broke almost all tests instead of one becausechar *sodium_version_string()
return value was misinterpreted (1.0.11 chunk_size > (size_t) 0U stream.nonce != (uint64_t) 0U /dev/random ret == 0 /dev/urandom randombytes/salsa20/randombytes_salsa20_random.c
instead of just1.0.11
). And this string built frombyte[]
/char[]
somewhere injnr-ffi
does containNUL
char at position6
.As could be see from hexdump resulting java string ends where binary contains
NUL
-byte twice first time in stream.Env:
To reproduce get code from #107 and run
mvn clean test -DargLine=-Dfile.encoding=windows-1251