[BUG][C] The bundled cJSON is an old version, and it should be a library dependency anyway

[imaami]

Currently cJSON is just a copy of the v1.7.7 sources (with a few typo fixes in comments). It's old (2018) and not used like a library dependency really should. It should be dynamically linked like is already done with OpenSSL and libcurl.

Word diff of upstream cJSON v1.7.7 and openapi-generator's bundled copy:

Here's the script from the screenshot:

#!/usr/bin/env bash

domain='https://raw.githubusercontent.com'
a="$domain/DaveGamble/cJSON/refs/tags/v1.7.7"
b="$domain/OpenAPITools/openapi-generator/refs/heads/master"
b+='/modules/openapi-generator/src/main/resources/C-libcurl'

for f in cJSON.{c,h}; do
  printf -vx "%0${#f}d"
  x="+-${x//0/-}-+"
  printf "\n$x\n| %s |\n$x\n" "$f"
  git diff -U0 --no-index --color=always --color-words \
      <(curl "$a/$f" -s) <(curl "$b/$f.mustache" -s) \
  | tail -n+4
done

echo

[eafer]

I understand the point, but I'm worried we'll end up having to tell the users to build cJSON themselves, since it doesn't seem to be as widely available as curl. On macos for example, it would seem to require homebrew. Also the project itself endorses copying the source like this: https://github.com/DaveGamble/cJSON?tab=readme-ov-file#copying-the-source.

Maybe we could just upgrade to a newer version? (Assuming that we have bugs here)

[imaami]

I understand the point, but I'm worried we'll end up having to tell the users to build cJSON themselves, since it doesn't seem to be as widely available as curl. On macos for example, it would seem to require homebrew.

I don't think that's the case. On Linux libcjson is definitely in every package repository out there. The GitHub macOS runner already has Homebrew installed by default, and the C examples link against the Homebrew OpenSSL:

-- The C compiler identification is AppleClang 15.0.0.15000309
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /Applications/Xcode_15.4.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/cc - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Found OpenSSL: /opt/homebrew/Cellar/openssl@3/3.4.0/lib/libcrypto.dylib (found version "3.4.0")
-- Found CURL: /Applications/Xcode_15.4.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX14.5.sdk/usr/lib/libcurl.tbd (found suitable version "8.6.0", minimum required is "7.58.0")
-- Configuring done (5.1s)
-- Generating done (0.0s)
-- Build files have been written to: /Users/runner/work/openapi-generator/openapi-generator/samples/client/petstore/c/build/Debug

Also the project itself endorses copying the source like this: https://github.com/DaveGamble/cJSON?tab=readme-ov-file#copying-the-source.

I don't personally interpret that as an endorsement. It reads more like a "yes, it's OK" note to those who prefer to copy the source files. But this is splitting hairs anyway, as any Open Source library can be copied as source just as well.

Maybe we could just upgrade to a newer version? (Assuming that we have bugs here)

Regardless of every other decision: yes, the first thing should be to just update the current sources in the repo. There are even CVEs in the current old version.

---

Side note about cJSON: I don't like it that much. Or maybe "like" is the wrong verb: I don't trust it. It doesn't actually comply with the JSON specification because its UTF-8 parsing is broken.

And then there are other issues. For example its development seems to be nearly dead right now, and one of the last PRs that was accepted into is Undefined Behavior: DaveGamble/cJSON#924

...but I'm not suggesting it should be replaced with some other library. I don't know what would be a better choice, I haven't vetted all of the available ones. Just wanted to point this out.

[eafer]

The GitHub macOS runner already has Homebrew installed by default

Ok, but I think most macos installations don't, and some users may not want to install it. Also my point isn't really about macos, that was just an example. I just don't know where this thing will be built or run, so I'd rather not add a new dependency if it can be helped.

the C examples link against the Homebrew OpenSSL

Openssl is an optional dependency and it only seems to be used for base64 encoding/decoding, so plenty of things should still work without it. (By the way, I find it excessive to have a dependency for that alone, we could just reimplement base64 and get rid of openssl entirely.)

Side note about cJSON: [...] its development seems to be nearly dead right now, and one of the last PRs that was accepted into is Undefined Behavior

I agree that this isn't a great look for cjson; but if the project is indeed dead, then that means there's pretty much no advantage to dynamic linking.

[imaami]

(By the way, I find it excessive to have a dependency for that alone, we could just reimplement base64 and get rid of openssl entirely.)

Oh definitely. Base64 is not that complicated.