URL:
https://github.com/SSSD/sssd/pull/542
Title: #542: KCM: Use json_loadb() when dealing with sss_iobuf data
fidencio commented:
"""
@jhrozek, @pbrezina,
Without looking at the code I'd give a +1 to Pavel's suggestion as well. However,
taking a look at the code makes me quite reluctant to go for it.
Let me try to explain my reasons and then I'll just wait to be convinced the opposite
(and here I'm hoping it'll happen):
- I do **not** know what kind of data is being passed to tcurl_http_send():
Without knowing exactly what has been passed to tcurl_http_send(), I sincerely have no
clue on how to determine whether this data contains or not a '\0' in the end.
Without knowing it, it's really hard to know when and where to make use of
`sss_iobuf_get_data_as_string()`.
I hope that you guys have a better clue about these details and will help me to have a
better idea as well (a sheet with all the data we receive and whether those are NULL or
non-NULL terminated would be what I'm looking for here).
- The current code adds NULL terminated strings to `sss_iobuf`:
Shall these change as well? Because if we're adding a new method to explicitly treat
bytes as bytes and strings as strings ... we should start by **only** adding bytes content
to sss_iobuf.
I do believe this has been happening because the data passed to the write_callback of
curl is explicitly **non** NULL terminated, as explicitly said here:
https://curl.haxx.se/libcurl/c/CURLOPT_WRITEFUNCTION.html
- Is all those changes worth it considering
https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahoste...):
I'm asking this because the suggestion given, although it looks simple, would take
me a quite long time to implement (which may not be the case for someone who's more
used to this code) ... and it sincerely feels like we'd be adding a new method and
changing a lot of things to conform with one specific place.
So, please, help me to understand better your ideas and then I'll get back to this.
In any case, I'd really like to suggest:
- Let's have this patch merged and have the fixes backported all over the place;
- Let's open a ticket for the changes (kudos if the questions raised here are all
answered there);
"""
See the full comment at
https://github.com/SSSD/sssd/pull/542#issuecomment-375789900