Skip to content

Commit

Permalink
Fix incorrect stripping of literal dollar sign in regexps (#6116)
Browse files Browse the repository at this point in the history
* Fix incorrect stripping of '$' from the regexp '\$' (literal dollar sign)

* Add a workaround to allow querying the test program for '--help'
without requiring test data. The issue was raised at
mirage/alcotest#358

* Update changelog

* Add link to alcotest issue

* Clarify comment

* Rephrase
  • Loading branch information
mjambon authored and brandonspark committed Sep 22, 2022
1 parent 8ea26d0 commit bf969ae
Show file tree
Hide file tree
Showing 5 changed files with 36 additions and 13 deletions.
2 changes: 2 additions & 0 deletions changelog.d/gh-5987.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Fixed incorrect stripping of '\$' (literal dollar sign) in regexps used in the
context of `metavariable-regex`.
5 changes: 5 additions & 0 deletions semgrep-core/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,11 @@ clean:
#
.PHONY: test
test: all
# The test executable has a few options that can be useful
# in some contexts.
# The following command ensures that we can call 'test.exe --help'
# without having to chdir into the test data folder.
./_build/default/tests/test.exe --help 2>&1 >/dev/null
$(MAKE) -C src/spacegrep test
dune runtest -f --no-buffer

Expand Down
19 changes: 9 additions & 10 deletions semgrep-core/src/utils/Regexp_engine.ml
Original file line number Diff line number Diff line change
Expand Up @@ -129,8 +129,9 @@ let remove_end_of_string_assertions_from_string src : string option =
match (a0, a1) with
| '^', '$' -> Some ""
| '^', c -> String.make 1 c |> finish
| c, '$' -> String.make 1 c |> finish
| '\\', ('A' | 'Z' | 'z') -> Some ""
| '\\', _ -> Some src
| c, '$' -> String.make 1 c |> finish
| _, _ -> src |> finish
else
(* "XXX" or longer *)
Expand All @@ -140,23 +141,21 @@ let remove_end_of_string_assertions_from_string src : string option =
| '\\', 'A' -> String.sub src 2 (len - 2)
| _ -> src
in
(* "X" or longer *)
(* remaining string: "X" or longer *)
let len = String.length src in
let z1 = src.[len - 1] in
if len = 1 then
match z1 with
| '$' -> Some ""
| _ -> src |> finish
else
(* "XX" or longer *)
(* remaining string: "XX" or longer *)
let z0 = src.[len - 2] in
let src =
match (z0, z1) with
| _, '$' -> String.sub src 0 (len - 1)
| '\\', ('Z' | 'z') -> String.sub src 0 (len - 2)
| _ -> src
in
finish src
match (z0, z1) with
| '\\', ('Z' | 'z') -> Some (String.sub src 0 (len - 2))
| '\\', _ -> Some src
| _, '$' -> String.sub src 0 (len - 1) |> finish
| _ -> src |> finish

let remove_end_of_string_assertions (src_pat, _old) =
match remove_end_of_string_assertions_from_string src_pat with
Expand Down
9 changes: 7 additions & 2 deletions semgrep-core/src/utils/Unit_regexp_engine.ml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,11 @@ let test_remove_eos_assertions () =
("AA", Some "AA");
("AAA", Some "AAA");
("AAAA", Some "AAAA");
({|\$|}, Some {|\$|});
({|A\$|}, Some {|A\$|});
({|AA\$|}, Some {|AA\$|});
({|AAA\$|}, Some {|AAA\$|});
({|AAAA\$|}, Some {|AAAA\$|});
("^", Some "");
("$", Some "");
({|\A|}, Some "");
Expand All @@ -30,10 +35,10 @@ let test_remove_eos_assertions () =
("^AAA$", Some "AAA");
("^^", None);
("$$", None);
({|A\A|}, None);
({|A\A|}, Some {|A\A|});
("[$]*", None);
("(?:^)", None);
({|\\A|}, None);
({|\\A|}, Some {|\\A|});
({|(?<!.|\n)|}, None);
(* DIY beginning-of-string assertion = \A *)
({|(?!.|\n)|}, None) (* DIY end-of-string assertion = \z *);
Expand Down
14 changes: 13 additions & 1 deletion semgrep-core/tests/Test.ml
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,20 @@ let tests () = List.flatten [
(* Entry point *)
(*****************************************************************************)

(*
This allows running the test program with '--help' from any folder
without getting an error due to not being able to load test data.
See https://github.com/mirage/alcotest/issues/358 for a request
to allow what we want without this workaround.
*)
let tests_with_delayed_error () =
try tests ()
with e ->
["cannot load test data - not a real test", (fun () -> raise e)]

let main () =
let alcotest_tests = Testutil.to_alcotest (tests ()) in
let alcotest_tests = Testutil.to_alcotest (tests_with_delayed_error ()) in
Alcotest.run "semgrep-core" alcotest_tests

let () = main ()

0 comments on commit bf969ae

Please sign in to comment.