Strip quotes from values

[bostjan]

Hello,

I was just trying out your library and I have noticed that returned values include quotes if quotes exist in ini file, which is a tad annoying behavior for my use case.

I understand that to existing users this behavior might be expected and preferred.

I wonder if patch that creates new -DSTRIP_VALUE_QUOTES would be accepted and merged?

Thanks for your consideration,
b.

[benhoyt]

Hi @bostjan, thanks for the suggestion. I've thought about this, and decided I probably wouldn't accept a patch for simply stripping quotes. A couple of reasons:

1. inih is (loosely) based on and compatible with Python's INI file parser (configparser), which doesn't support this. The quoted strings simply include the quotes, just like inih:

>>> import configparser, io
>>> config = configparser.ConfigParser()
>>> config.read_file(io.StringIO('[section]\nquoted = "this is a test"\n'))
>>> config['section']['quoted']
'"this is a test"'

2. The goal if inih is to be simple, and handling quotes properly quickly gets complicated. You can't really just strip a quote character at the beginning and end, because you need to consider escaping as well (which itself quickly gets complicated, e.g., do you need support for \", \n, \t, \u201C, etc?).

So I suggest simply stripping the quotes yourself in your own code/handler. For example, maybe something like this (though this is untested code!):

// Strip " character from beginning and end of value -- doesn't handle escapes
if (value[0] == '"') {
    value++;
    if (value[0] && value[strlen(value) - 1] == '"') {
        value[strlen(value) - 1] = '\0';
    }
}

[bostjan]

Hey Ben, thanks for the answer and suggestion.

I can live with a patch, which I already created in the mean time (handles single quotes too):
https://github.com/a2o/snoopy/blob/bugfix/github-106-nm-segfault/lib/inih/patches/0001-strip-value-quotes.diff

Will review your code too, it is probably more elegant than mine (C is an occasional hobby for me).

I anticipated the escaping part too, and I guess I will not go that far - KISS FTW.

Feel free to close this issue at your leisure.

[benhoyt]

Hi @bostjan Thanks -- note that you wouldn't have to patch ini.c, you could simply add the "quote skipping" code to the top of your user handler. I think the approach you're taking in your patch is reasonable, however, I don't understand why you have the same test in the first if and then again in the else if?

[bostjan]

@benhoyt

My use case for inih was to replace ndevilla/iniparser, which was doing very well until users reported segfaults under certain conditions, which I have no desire to investigate TBH.

Patching ini.c is simpler in my case, as it does less code pollution in the original project (which assumed surrounding quotes would be removed already when previous library was used). But yes, agreed, your suggestion would work too. Will think about it, thanks.

About else if:
Value is either surrounded with double, or with single quotes, and not both. Removing both double and surrounded single quotes at the same time would really not make any sense.

Example (I know, convoluted and stupid, but proves the point):
adsf = "'bsdf' == 'csdf'"

[benhoyt]

Regarding the double quotes vs single quotes -- in that case I think you'll want to change the second one to look for the single quote character:

if ((*value == '\'') && (value[strlen(value) - 1] == '\'')) {

Closing this issue now.

[bostjan]

Aaaaah! And when I was working on that, I knew I had missed something and it was itching in my subconscious. Thanks!