bug fix: new ares_strsplit#492
Merged
bradh352 merged 2 commits intoc-ares:mainfrom Dec 13, 2022
createyourpersonalaccount:new-strsplit
Merged
bug fix: new ares_strsplit#492bradh352 merged 2 commits intoc-ares:mainfrom createyourpersonalaccount:new-strsplit
bradh352 merged 2 commits intoc-ares:mainfrom
createyourpersonalaccount:new-strsplit
Conversation
The test reveals a bug in the implementation of ares_strsplit when the
make_set parameter is set to 1, as distinct domains are confused for
equal:
out = ares_strsplit("example.com, example.co", ", ", 1, &n);
evaluates to n = 1 with out = { "example.com" }.
The purpose of ares_strsplit in c-ares is to split a comma-delimited
string of unique (up to letter case) domains. However, because the
terminating NUL byte was not checked in the substrings when comparing
for uniqueness, the function would sometimes drop domains it should
not. For example,
ares_strsplit("example.com, example.co", ",")
would only result in a single domain "example.com".
Aside from this bugfix, the following cleanup is performed:
1. The tokenization now happens with the help of strcspn instead of the
custom function is_delim.
2. The function list_contains has been inlined.
3. The interface of ares_strsplit has been simplified by removing the
parameter make_set since in practice it was always 1.
4. There are fewer passes over the input string.
5. We resize the table using realloc() down to its minimum size.
6. The docstring of ares_strsplit is updated and also a couple typos
are fixed.
There occurs a single use of ares_strsplit and since the make_set
parameter has been removed, the call in ares_init.c is modified
accordingly. The unit test for ares_strsplit is also updated.
bagder
approved these changes
Nov 8, 2022
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The first commit adds a unit test that reveals a bug in
ares_strsplitand the second commit fixes the algorithm.