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

gsub with empty regex doesn't terminate #1206

Closed
4z3 opened this issue Aug 11, 2016 · 4 comments · Fixed by #2641
Closed

gsub with empty regex doesn't terminate #1206

4z3 opened this issue Aug 11, 2016 · 4 comments · Fixed by #2641
Labels
Milestone

Comments

@4z3
Copy link

4z3 commented Aug 11, 2016

Calling gsub with an empty regex will run forever. Example:

$ jq -n '"" | gsub(""; "")'

Not sure if this is a bug or expected behavior, but terminating with an error might be more appropriate :)

@pkoppstein
Copy link
Contributor

The bug was fixed by ad8d1a8 around Dec 9, 2015; see #1033

That is, it was fixed AFTER the release of jq 1.5

@4z3
Copy link
Author

4z3 commented Aug 12, 2016

I can reproduce this bug with current master (0b82185).

@agguser
Copy link

agguser commented Apr 26, 2022

Seems that empty matches also cause this bug.

$ jq -n -r '"abc\nabc" | gsub("^"; "+")'   # not terminate!

$ jq -n -r '"abc\nabc" | sub("^"; "+")'   # work if not 'g'
+abc
abc

$ jq -n -r '"abc\nabc" | gsub("^."; "+")'   # weird!
+++
abc

@itchyny
Copy link
Contributor

itchyny commented Apr 26, 2022

Related issue: #2148

@itchyny itchyny added the bug label Jun 3, 2023
pkoppstein added a commit to pkoppstein/jq that referenced this issue Jun 29, 2023
… uniq(stream)

The primary purpose of this commit (which supercedes PR
jqlang#2624) is to rectify most problems
with `gsub` (and also `sub` with the "g" option), in particular jqlang#1425
('\b'), jqlang#2354 (lookahead), and jqlang#2532 (regex == "^(?!cd ).*$|^cd ";"")).

This commit also partly resolves jqlang#2148 and jqlang#1206 in that `gsub` no
longer loops infinitely; however, because the new `gsub` depends
critically on match(_;"g"), the behavior when regex == "" is sometimes
non-standard. [*1]

Since the new sub/3 relies on uniq/1, that has been added as well [*2].

The documentation has been updated to reflect the fact that `sub` and
`gsub` are intended to be regular in the second argument. [*3]

Also, _nwise/1 has been tweaked to take advantage of TCO.

Footnotes:

[*1] Using the new gsub, '"a" | gsub( ""; "a")' emits "aa" rather than
"aaa" as would be standard.  This is nevertheless better than the
infinite loop behavior of jq 1.6 in this case.

With one exception (as explained in [*2]), the new gsub is implemented
as though match/2 behavior is correct.  That is, bugs in `gsub`
behavior will most likely have their origin in `match/2`.

[*2] `uniq/1` adopts the Unix/Linux name and semantics; it is needed for the following test case:

gsub("(?=u)"; "u")
"qux"
"quux"

Without this functionality:

Test jqlang#23: 'gsub("(?=u)"; "u")' at line number 100
*** Expected "quux", but got "quuux" for test at line number 102: gsub("(?=u)"; "u")

The root of the problem here is `match`: if `match` is fixed, then gsub would not need `untie`.

The addition of `uniq` as a top-level function should be a non-issue
relative to general concern about builtins.jq bloat: the line count of
the new builtin.jq is significantly reduced overall, and the number of
defs is actually reduced by 1 (from 111 (ignoring a redundant def) to 110).

[*3] See e.g. jqlang#513 (comment)
@itchyny itchyny added this to the 1.7 release milestone Jul 2, 2023
itchyny pushed a commit that referenced this issue Jul 3, 2023
The primary purpose of this commit is to rectify most problems with
`gsub` (and also `sub` with the `g` option), in particular fix #1425 ('\b'),
fix #2354 (lookahead), and fix #2532 (regex == `"^(?!cd ).*$|^cd "`).

This commit also partly resolves #2148 and resolves #1206 in that
`gsub` no longer loops infinitely; however, because the new `gsub`
depends critically on `match/2`, the behavior when regex == `""` is
sometimes non-standard.

The documentation has been updated to reflect the fact that `sub`
and `gsub` are intended to be regular in the second argument.

Also, `_nwise/1` has been tweaked to take advantage of TCO.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants