Add --json-file/-j option to read from file w/ json contents #311
Conversation
brew install...
not leaking
cleanup action cleanup trurl.1 cleanup trurl.1
|
okay I need to figure out whats going on with the windows builds, the cygwin was was working on my local branch until very recently, might be something i changed on the way we are doing the file i/o. I will take a closer look this evening. Edit: Okay, i got the cygwin build working. I just need to add libjson-c to the curl-for-win build. @vszakats would this be done by manually compiling and linking it in here: https://github.com/curl/curl-for-win/blob/main/trurl.sh ? |
|
It's a new dependency, kind of a big deal to add support for one in general. One approach is to make it a build option, disabled by default, so existing trurl builds (including curl-for-win) continue to work without modification. Another option is to delete/disable the curl-for-win CI job till I (or someone else) get around adding support for it. A 3rd one is to add an option to disable this feature, and I update curl-for-win to set it. |
|
...that said, have you thought of using a JSON parser that's a self-contained C89 source (with a fitting license)? It's still a dependency, but perhaps easier to integrate and carry. |
I was looking a lot at https://github.com/DaveGamble/cJSON, but I didn't like how it looks and json-c has some nice features. Both this and json-c are in vcpkg, freebsd repos, and most linux distro repositories so i was hoping it wouldn't be too much work either way for maintainers. If folks want a smaller ANSI c json library instead of the one I chose, I think that's a fair request and I can start working this again. for now, I will put it behind a compile time flag as you suggested Edit: |
Co-authored-by: Viktor Szakats <vszakats@users.noreply.github.com>
Co-authored-by: Viktor Szakats <vszakats@users.noreply.github.com>
Co-authored-by: Viktor Szakats <vszakats@users.noreply.github.com>
There was a problem hiding this comment.
We made the -f function possible to work as a filter, as in it reads URLs and outputs the results in the mean time. This JSON reader reads the entire file before it acts on it, which makes it not work as a filter and if you want to work an a huge number of JSON URLs this will allocate a lot of memory...
trurl.c
Outdated
| while((n = fread(json_string + latest, sizeof(char), json_buf_size, file))) { | ||
| latest += n; | ||
| json_buf_size *= 2; | ||
| char *new_json_string = realloc(json_string, json_buf_size); |
There was a problem hiding this comment.
There is something wrong in the math here.
First loop entry: latest is 0, json_buf_size is 1024.
It loads 1024 bytes. latest is bumped to 1024. json_buf_size is doubled to 2048.
The buffer is realloced to be 2048 bytes.
Now loop.
The fread reads data into offset 1024 in the buffer that is 2048 bytes big. But fread is asked to read 2048 bytes, meaning that it will write outside the buffer if the file is larger than 2048 bytes.
Or am I wrong?
I think you should rather convert this to read data into a stack based fixed size buffer, then copy that data to the end of a realloced buffer of the appropriate size.
There was a problem hiding this comment.
I think that's a better approach. I was pretty unhappy with this in general (I tried I to rework it but it broke on windows lol) Along with your comment about making it act more like -f I will need to redo this.
It would be ideal if it only allocated the memory required for the current url in the array, which shouldnt be too hard.
There was a problem hiding this comment.
A way to get it to behave more like -f, would be to have it scan for matching { and } (with proper checking you aren't in a string a what not) and when you've go a full enclosed object it would take that and parse it and perform the trurl operations on it. this would make it so it only allocates as much memory as required for the current buffer. the big issue i see with this is that trurl could parse non-conformant json unless we bake all that parsing logic for arrays in as well (at the point how far would we be from an actual json parser?).
|
@bagder, i've made a few big modifications. Mainly I made it so it reads one json object at a time, so it now only allocates as much memory as required to parse the largest single json object, instead of attempting to read until the of the file. I also made it so it reads in a fixed buffer on the stack, and then allocates more data on the heap if need be as you suggested. |
Co-authored-by: Viktor Szakats <vszakats@users.noreply.github.com>
This adds
--json-fileas an option to read from a file that contains a json representation of the urls. It works very similarly to--url-file. The--json-fileinput structure is designed to match roughly what the output of--jsonproduces. the simplest json object possible is:You can pass an arbitrary number of these url JSON objects and perform operations on them like you would with any other url in trurl.
Something that I did that may be up for debate is how the query is handled. I chose to make the query only be able to be set by an array of "keys" and "values" (the "params" section of --json") and it will throw a warning if the "query" component of "parts" is set.
A few examples:
Currently the feature is disabled by default, do turn it on you need to add
TRURL_JSON_INto your environment to enable it.fixes: #291