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.

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

json_object_get_int does not set EINVAL on invalid string #792

Closed
fedefrancescon opened this issue Oct 24, 2022 · 2 comments
Closed

json_object_get_int does not set EINVAL on invalid string #792

fedefrancescon opened this issue Oct 24, 2022 · 2 comments

Comments

@fedefrancescon
Copy link

fedefrancescon commented Oct 24, 2022

Describe the bug
The doc states that json_object_get_int should set errno to EINVAL.
This is not the case! Looking at the code, in case the type is string and the parsing fails it returns 0 but errno is not set.

Steps To Reproduce
Compile the following code with no particular flags or settings:

#include <assert.h>
#include <json-c/json.h>
#include <stdio.h>
#include <errno.h>

int main(const int argc, const char *argv[]){
  const char vals[][10] = {
    "b",
    "0xb",
    "0xB",
    "0xB0",
    "0B",
    "pippo",
    "pluto"
  };

  for (size_t i = 0; i < sizeof(vals)/sizeof(vals[0]); i++) {
    struct json_object *jtmp = json_object_new_string(vals[i]);
    errno = 0;
    int r = json_object_get_int(jtmp);
    printf("s: %-10s => i: %3lld errno: %d\n", vals[i], r, errno);
    json_object_put(jtmp);
  }

  return 0;
}

Will give you that output:

s: b          => i:   0 errno: 0
s: 0xb        => i:   0 errno: 0
s: 0xB        => i:   0 errno: 0
s: 0xB0       => i:   0 errno: 0
s: 0B         => i:   0 errno: 0
s: pippo      => i:   0 errno: 0
s: pluto      => i:   0 errno: 0

Version and Platform

  • json-c version: 777dd06 , 0.15
  • OS: Arch Linux x64 , Kirkstone on aarch64

How to Proceed?
I was on my way to issue a PR to fix this (with related test), it's a really easy fix. But I noticed that there is some inconsistency within:

  • json_object_get_double
  • json_object_get_int
  • json_object_get_int64
  • json_object_get_uint64

int and double both state that invalid conversions should set errno = EINVAL, double actually seems to do this!
int64 and uint64 do not state the same and simply (at first glance) returns 0 in case of invalid parsing.

So, before going on an pushing a PR. Which way should I go? Which is the indended behaviour?
Could I go on and fix the missing errno = EINVAL or just remove the errno setting statement from the doc?

@hawicz
Copy link
Member

hawicz commented Oct 26, 2022

The fix for this belongs in the json_parse_int64() function. That calls strtoll() with the expectation that it will set errno on failure, but the man page states that this is optional ("...may also set..."):

The implementation may also set errno to EINVAL in case no conversion was performed (no digits seen, and 0 returned).

so we'll need to set it explicitly to get consistent behavior.

hawicz added a commit that referenced this issue Oct 26, 2022
…64 fails, to match the docs for json_object_get_int.
@hawicz
Copy link
Member

hawicz commented Oct 26, 2022

Note that the errno: 0 result for the cases that start with an actual "0" are the intended behavior. In those, when the string is "coerced to a int" (as per the docs) the answer is 0 and the extra characters are supposed to be ignored.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants