Fixed handling of long strings when using INI_USE_STACK #148
Conversation
Undefined behavior occurred if the incoming .ini file contains a line that is longer than INI_MAX_LINE with the INI_USE_STACK option. In such a case, it is safe to abort parsing even if INI_STOP_ON_FIRST_ERROR is not set. Signed-off-by: Mikhail Khachayants <[email protected]>
|
I'm not sure about INI_STOP_ON_FIRST_ERROR and unit tests. I would appreciate a discussion on this. |
| @@ -156,6 +156,14 @@ int ini_parse_stream(ini_reader reader, void* stream, ini_handler handler, | |||
|
|
|||
| lineno++; | |||
|
|
|||
| #if INI_USE_STACK | |||
There was a problem hiding this comment.
I think we should do this check whether or not INI_USE_STACK is set, right? Because even in non-stack mode we might have read and realloc'd up to INI_MAX_LINE bytes.
| @@ -156,6 +156,14 @@ int ini_parse_stream(ini_reader reader, void* stream, ini_handler handler, | |||
|
|
|||
| lineno++; | |||
|
|
|||
| #if INI_USE_STACK | |||
| if (strlen(line) == max_line - 1 && line[max_line - 2] != '\n') { | |||
| /* Exit even if INI_STOP_ON_FIRST_ERROR is not set */ | |||
There was a problem hiding this comment.
Yeah, I'm not sure about this. If INI_STOP_ON_FIRST_ERROR is false (the default), I think we should read and discard the rest of the current line, and then proceed. We could decide to do this as a stop-gap fix, but I'm not sure that's a good idea -- if we fix this I think we should do it properly in a single PR.
You can see how it's breaking the existing max-line tests as it is currently.
|
OK, thanks for the comments. I'll rework my PR and will come back later. |
|
He never did lmao |
|
@tyler92 you still alive? :0 |
|
Alive, but too busy to make changes here. I will close this MR, feel free to open new one with my changes |
Undefined behavior occurred if the incoming .ini file contains a line that is longer than INI_MAX_LINE with the INI_USE_STACK option. In such a case, it is safe to abort parsing even if INI_STOP_ON_FIRST_ERROR is not set.
See #145 for the details
Signed-off-by: Mikhail Khachayants [email protected]