Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add length parameters to all the functions that take or return strings #57

Open
FSMaxB opened this issue Nov 12, 2016 · 10 comments
Open

Add length parameters to all the functions that take or return strings #57

FSMaxB opened this issue Nov 12, 2016 · 10 comments
Labels
enhancement
Milestone
2.0.0

Comments

@FSMaxB
Copy link
Collaborator

FSMaxB commented Nov 12, 2016 •
edited
Loading

I think all functions that take a string or return one should also take/return a length.

This would be part of the breaking changes in 2.0.0.
@FSMaxB FSMaxB added this to the 1.0.0 milestone Nov 12, 2016
@FSMaxB
Copy link
Collaborator Author

FSMaxB commented Nov 12, 2016

That would also allow cJSON to improve conformance with the JSON standard in the future by allowing '\0' as part of a string.

@FSMaxB FSMaxB mentioned this issue Nov 12, 2016
Closed
@FSMaxB
Copy link
Collaborator Author

FSMaxB commented Mar 19, 2017

I am currently refactoring the parser so it doesn't rely on '\0' as end marker for strings anymore.

@maluJ
Copy link

maluJ commented Mar 28, 2018

@FSMaxB Is this feature implemented in the newest version of cJSON? Or does it still rely on null terminating strings? It would be extremly helpful for us if there was a parse function that takes a length as argument.

@FSMaxB
Copy link
Collaborator Author

FSMaxB commented Mar 28, 2018

@maluJ It will still take some time until this makes it into a release, but internally it is already implemented. My current plan ist to expose it with #186, then make a release candidate and wait for feedback, but I can't provide any estimate on when this will be the case.

If you want this now and don't mind maintaining a local fork, you can make your own version of cJSON_ParseWithOpts that doesn't use strlen but your own parameter in line 1016:

cJSON/cJSON.c

Lines 1000 to 1078 in 0e0c463

/* Parse an object - create a new root, and populate. */
CJSON_PUBLIC(cJSON *) cJSON_ParseWithOpts(const char *value, const char **return_parse_end, cJSON_bool require_null_terminated)
{
parse_buffer buffer = { 0, 0, 0, 0, { 0, 0, 0 } };
cJSON *item = NULL;
/* reset error position */
global_error.json = NULL;
global_error.position = 0;
if (value == NULL)
{
goto fail;
}
buffer.content = (const unsigned char*)value;
buffer.length = strlen((const char*)value) sizeof("");
buffer.offset = 0;
buffer.hooks = global_hooks;
item = cJSON_New_Item(&global_hooks);
if (item == NULL) /* memory fail */
{
goto fail;
}
if (!parse_value(item, buffer_skip_whitespace(skip_utf8_bom(&buffer))))
{
/* parse failure. ep is set. */
goto fail;
}
/* if we require null-terminated JSON without appended garbage, skip and then check for a null terminator */
if (require_null_terminated)
{
buffer_skip_whitespace(&buffer);
if ((buffer.offset >= buffer.length) || buffer_at_offset(&buffer)[0] != '\0')
{
goto fail;
}
}
if (return_parse_end)
{
*return_parse_end = (const char*)buffer_at_offset(&buffer);
}
return item;
fail:
if (item != NULL)
{
cJSON_Delete(item);
}
if (value != NULL)
{
error local_error;
local_error.json = (const unsigned char*)value;
local_error.position = 0;
if (buffer.offset < buffer.length)
{
local_error.position = buffer.offset;
}
else if (buffer.length > 0)
{
local_error.position = buffer.length - 1;
}
if (return_parse_end != NULL)
{
*return_parse_end = (const char*)local_error.json local_error.position;
}
global_error = local_error;
}
return NULL;
}

@simias
Copy link

simias commented Jun 15, 2018

I'm in the same boat as @maluJ, it's too bad that currently cJSON_Parse and friends don't expose the length since I'm receiving not-NULL terminated JSON strings that I want to parse. I'm going to use the modification you're proposing for the time being, thanks.

@FSMaxB
Copy link
Collaborator Author

FSMaxB commented Jun 15, 2018

@simias Sadly I've been really busy lately, so I didn't find the time to do much more than fix bugs and respond to issues.

@gdamore
Copy link

gdamore commented Sep 10, 2018

I was looking around, and thought, surely this must be done now. Really all that is needed is just an alternative version of ParseWIthOpts that passes a length.

I can build a local version that does this and send a PR, if that's more likely to get the change integrated?

@MopeZe
Copy link

MopeZe commented Sep 11, 2018

@FSMaxB @gdamore It would still be a great improvement if a function with len argument would be available. So yes, would be great if you can file a PR.

@gdamore
Copy link

gdamore commented Sep 11, 2018 via email

@gdamore gdamore mentioned this issue Sep 11, 2018
Closed
@gdamore
Copy link

gdamore commented Sep 11, 2018

Please see PR #299

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
2.0.0
Development

No branches or pull requests
5 participants
@gdamore @FSMaxB @simias @maluJ @MopeZe