This PR modifies `replace_all` to avoid the infinite loop. In each iteration, `replace_all` searches for `needle` starting from the beginning of `haystack`. If `replacement` contains `needle`, the result is an infinite loop. To prevent this from happening, `replace_all` should continue searching for `needle` from the end of the previous `replacement`.
Squashed and rebased from #1128. Resolves #936. You can view, comment on, or merge this pull request online at:
https://github.com/geany/geany-plugins/pull/1232
-- Commit Summary --
* Markdown: Modify replace_all to avoid infinite loop
-- File Changes --
M markdown/src/viewer.c (7)
-- Patch Links --
https://github.com/geany/geany-plugins/pull/1232.patch https://github.com/geany/geany-plugins/pull/1232.diff
@b4n commented on this pull request.
Not tested, but it looks good from here.
So... waiting until 2025, when Ubuntu 20.04 is EOL, so can use [g_string_replace()](https://github.com/geany/geany-plugins/pull/1128#issuecomment-954651043)?
LGBI but someone needs to test it.
The problem is that the markdown plugin has been in and out of plugins due to its delayed GTK3 port so its not clear how many users there are, and its likely to go away again due to wekkit2gtk-4.1 (see https://github.com/geany/geany-plugins/pull/1295#issuecomment-2073809698) unless somebody makes necessary changes.
@xiota It needs an active maintainer who can then make decisions like when to swap to new functions even if it makes the plugin not build on older systems, but since you have fixed it without that then yeah probably should wait.
@elextr In a comment somewhere, you mention you're on an LTS distro without access to webkit2gtk-4.1? So I can look up what libraries you have available, what distro and release is it?
IIUC any Ubuntu 20.04 LTS or derivative (eg Linux Mint 20) does not have webkit2gtk-4.1.
So... checking availability of any library on the oldest Ubuntu LTS release should be sufficient?
For webkit2gtk, there are a couple discontinued functions that probably need to be updated. Ubuntu 20.04 could be supported with conditional compilation, but if there will be no new geany/plugins release before 20.04 is EOL, there wouldn't be much point.
There were already some mumblings about a point release due to fixes of potential crashes being merged IIUC, and hopefully the next release won't be as far away as 2.0 was from 1.38, but its hard to tell.
@b4n approved this pull request.
I extracted the old and new code an tested a bit, and that also works great. FWIW, if somebody didn't catch what the issue was (as in fact it's not mentioned anywhere but "infinite loop"): if the replacement contains the search term, it'll be replaced recursively forever (infinite loop + steady memory growth).
<details> <summary>FWIW, I tested using this program:</summary>
```c #include <stdlib.h> #include <stdio.h> #include <string.h> #include <glib.h>
#if 0 /* old */ static void replace_all(GString *haystack, const gchar *needle, const gchar *replacement) { gchar *ptr; gsize needle_len = strlen(needle);
/* For each occurrence of needle in haystack */ while ((ptr = strstr(haystack->str, needle)) != NULL) { goffset offset = ptr - haystack->str; g_string_erase(haystack, offset, needle_len); g_string_insert(haystack, offset, replacement); } } #else /* new */ static void replace_all(GString *haystack, const gchar *needle, const gchar *replacement) { gchar *ptr; gsize needle_len = strlen(needle); gsize replacement_len = strlen(replacement); goffset offset = 0;
/* For each occurrence of needle in haystack */ while ((ptr = strstr(haystack->str + offset, needle)) != NULL) { offset = ptr - haystack->str; g_string_erase(haystack, offset, needle_len); g_string_insert(haystack, offset, replacement); offset += replacement_len; } } #endif
int main(void) { #define TEST_REPLACE_ALL(hs, s, r, ex) \ G_STMT_START { \ GString *haystack_ = g_string_new(hs); \ replace_all(haystack_, s, r); \ g_assert_cmpstr(haystack_->str, ==, ex); \ g_string_free(haystack_, TRUE); \ } G_STMT_END
TEST_REPLACE_ALL("hello", "a", "b", "hello"); TEST_REPLACE_ALL("hallo", "a", "b", "hbllo"); TEST_REPLACE_ALL("foobarbaz", "ba", "BA", "fooBArBAz"); TEST_REPLACE_ALL("ababab", "ba", "BA", "aBABAb"); TEST_REPLACE_ALL("ababab", "ab", "AB", "ABABAB"); TEST_REPLACE_ALL("foobarbaz", "o", "oo", "foooobarbaz");
#undef TEST_REPLACE_ALL
return 0; } ```
@b4n said:
if the replacement contains the search term, it'll be replaced recursively forever (infinite loop + steady memory growth).
@xiota said:
replace_all searches for needle starting from the beginning of haystack. If replacement contains needle, the result is an infinite loop.
Those seem to say the same to me, and "infinite loop" (at least until it runs out of memory :-) sounds like the issue, sounds like furious agreement :-D
Oopsie, I didn't check the OP, only the commit message :confused:
Merged #1232 into master.
github-comments@lists.geany.org