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

PCRE RegEx broken #932

Open
djettah opened this issue Jul 12, 2023 · 6 comments · May be fixed by #1411
Open

PCRE RegEx broken #932

djettah opened this issue Jul 12, 2023 · 6 comments · May be fixed by #1411
Labels
bug Something isn't working

Comments

@djettah
Copy link

djettah commented Jul 12, 2023

Minimal .gitlab-ci.yml illustrating the issue

build-fake:
  stage: build
  variables:
    REGEX: '/^(\d+(\.\d+)*|latest)$/'
  script:
    - echo fake build
  rules:
    - if: $CI_COMMIT_TAG =~ $REGEX

Expected behavior
It should match a job but it doesn't:

# gitlab-ci-local --list --variable CI_COMMIT_TAG=1.11  
parsing and downloads finished in 56 ms
name        description  stage   when   allow_failure  needs

# gitlab-ci-local --list --variable CI_COMMIT_TAG=latest
parsing and downloads finished in 80 ms
name        description  stage   when        allow_failure  needs
build-fake               build   on_success  false      

Doesn't it support PCRE? GitLab does.

Host information
Ubuntu
gitlab-ci-local 4.41.2

Additional context
Add any other context about the problem here.

@firecow
Copy link
Owner

firecow commented Jul 14, 2023

We must support PCRE, but i don't think we do at the moment 👍

image

Something is definitely wrong 😃

@firecow firecow added the bug Something isn't working label Jul 14, 2023
@naweiss
Copy link
Collaborator

naweiss commented Jul 27, 2023

@firecow
Copy link
Owner

firecow commented Jul 27, 2023

Great, we need to utilize this package if possible then.
https://www.npmjs.com/package/re2

@naweiss
Copy link
Collaborator

naweiss commented Jul 27, 2023

@firecow I wish I would understood your first comment earlier. After digging into this subject for 3 hours I found out that the image you posted clearly shows that the specific regex mentioned in this issue should work using RegExp without the need for RE2 or PCRE . gitlab-ci-local handles the following case perfectly fine:

build-fake:
  stage: build
  script:
    - echo fake build
  rules:
    - if: $CI_COMMIT_TAG =~ /^(\d+(\.\d+)*|latest)$/

To be clear, yes we should support RE2 to allow things like '/^([[:digit:]]+(\.[[:digit:]]+)*|latest)$/' which are not supported by JS RegExp. But, there is another bug that causes the mentioned REGEX expansion to fail:

variable: (name) => JSON.stringify(envs[name] ?? null),

since REGEX is a variable it is replaced with JSON.stringify(envs[name]) which replaces every \ with \\. So, the evaluated result becomes: "1.11".match(/^(\\d+(\.\\d+)*|latest)$/) != null instead of "1.11".match(/^(\d+(\.\d+)*|latest)$/) != null.

@rgalonso
Copy link
Contributor

rgalonso commented Oct 1, 2024

This also affects coverage. I have a regex that produces the following error.

SyntaxError: Invalid regular expression: /(?i)(total.*? (100(?:\.0+)?\%|[1-9]?\d(?:\.\d+)?\%))$/: Invalid group
    at new RegExp (<anonymous>)
    at Function.getCoveragePercent (/snapshot/firecow-gitlab-ci-local/src/utils.ts:73:23)
    at Job.start (/snapshot/firecow-gitlab-ci-local/src/job.ts:551:37)
    at /snapshot/firecow-gitlab-ci-local/node_modules/p-map/index.js:57:22

Testing the pattern and test string on regex101.com confirms that it fails using ECMAScript but passes using PCRE2. Is there anything blocking having this project use RE2 instead of RegExp?

By the way, for those who read naweiss's earlier comment and find that the link no longer points to anything regex related, here's some current documentation for another GitLab feature (Push Rules) which confirms that GitLab is using RE2:

GitLab uses RE2 syntax for regular expressions in push rules. You can test them at the regex101 regex tester. Each regular expression is limited to 511 characters.

@ANGkeith
Copy link
Collaborator

ANGkeith commented Nov 5, 2024

original issue regarding broken regex should be fixed somewhere after 4.41.2...

image

@ANGkeith ANGkeith linked a pull request Nov 7, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
5 participants