LwM2M string resource is not a zero terminated C-string according to the LwM2M specification

[GardeningStevie]

I run into an issues with the current implementation of LwM2M String resource.

A string resource may be truncated with a zero-terminator when the buffer was used up entirely. In case of a UTF-8 multi-byte character as final symbol the result is an invalid UTF-8 string.

from lwm2m_engine_set(...)

	case LWM2M_RES_TYPE_STRING:
		if (len) {
			strncpy(data_ptr, value, len - 1);
			((char *)data_ptr)[len - 1] = '\0';
		} else {
			((char *)data_ptr)[0] = '\0';
		}
		break;

From the git history, the zero termination was always done in one or another way.

However, that is not in line with LwM2M specification which does specify that it is an UTF-8 string. In UTF-8, nul is a character like many other.

Many people, which are familiar with the C programming language, think that zero terminated strings are common sense. In reality, it is a concept of the the C programming language.

So, the current implementation which enforces zero-termination may cause compatibility issues with other LwM2M implementations. At least, any unit test should fail that do not get what he was set. That's the case with the current implementation.

My solution proposal
Since the datalen field is introduced, a String resource can be handled like Opaque. So, don't add or remove any character in the resource itself.

lwm2m_get_string() and lwm2m_set_string() can do the C string <-> LwM2M string conversion by adding and removing the C string zero terminator.

Actually, lwm2m_get_string() and lwm2m_get_opaque() functions lack also the information about the actual resource length. So, I added a parameter to get the information about the resource size.
So, the API of these functions breaks compatibility. Anyway, lwm2m_get_opaque() had no useful purpose without that information. lwm2m_get_string() adds now the C zero-termination but requires a buffer that can
take that additional character.

The first commit is too big. I cleaned some code that hurts my eyes. It was not really necessary.

However, any thoughts about the basic topic?

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
1.3% Duplication on New Code

See analysis details on SonarQube Cloud

[rlubos]

Looks reasonable I think. Thanks for adding extra tests!

[rlubos]

While the change makes sense, IMHO it'd be better to put it in a separate commit for clarity, the fix isn't really related to the string representation cleanup.

[rlubos]

Kind of weird we had function like this, how in the world would the user know how large the actual data content is in the buffer.

But anayway, it's still an API change - LwM2M API is unstable, but it should be covered in the migration guide for 4.3, please add an entry, can be in a separate commit.

[SeppoTakalo]

I fail to see how you can produce truncated stings into LwM2M engine.
You should get -ENOMEM if the buffer is too small.

static int lwm2m_check_buf_sizes(uint8_t data_type, uint16_t resource_length, uint16_t buf_length)
{
	switch (data_type) {
	case LWM2M_RES_TYPE_OPAQUE:
	case LWM2M_RES_TYPE_STRING:
		if (resource_length > buf_length) {
			return -ENOMEM;
		}
		break;

[SeppoTakalo]

By the way, referring to the LwM2M specification on this case does not apply.

LwM2M specifies the network protocol, nothing about the internal representation in the engine or the C API of the client.

So if something is not encoded correctly according to LwM2M spec, the content-type encoder/decoder needs to be fixed. We should not touch the API because of that.