Avoid catastrophic backtracing in regex #1690

Ravlen · 2021-03-10T07:55:47Z

@tancnle This PR attempts to fix some regex that could cause catastrophic backtracing, could you take a look?

For the `ghc_core` lexer:

For the `factor` lexer:

Ravlen · 2021-03-11T08:04:16Z

lib/rouge/lexers/factor.rb

@@ -243,7 +243,7 @@ def self.builtins
        end

        # strings
-        rule %r/"""\s+.*?\s+"""/, Str
+        rule %r/"""[^"].*?[^"]"""/, Str


Actually, I think this is wrong, let me fix that. Though I'm finding it hard to find what is actually supported in the Factor language. Does it need spaces after the triple quotes to be valid? Can it be split across multiple lines like this?

Test in GitHub:

article "ARTICLES" { { "title" "TITLE" { VARCHAR 256 } +not-null+ +user-assigned-id+ } { """title""" "TITLE" { VARCHAR 256 } +not-null+ +user-assigned-id+ } { ""title"" "TITLE" { VARCHAR 256 } +not-null+ +user-assigned-id+ } { """ title """ "TITLE" { VARCHAR 256 } +not-null+ +user-assigned-id+ } { """title over two lines to test it""" "TITLE" { VARCHAR 256 } +not-null+ +user-assigned-id+ } } define-persistent : <article> ( title -- article ) article new swap >>title ; TUPLE: revision id title author date content description ;

Maybe it's not even supported:

https://github.com/factor/factor/blob/master/core/strings/strings-tests.factor

https://docs.factorcode.org/content/article-strings.html

Interesting. Looks like GitHub is parsing the code correctly'ish 🤔 The highlight for double quotes seems to be out of place but overall it looks good.

I was about to suggest lifting the regexes directly from Pygments. However, it seems Pygments 2.8.1 (latest) - the Python package that Rouge compatible with - does not handle triple quotes well (could be an existing bug).

According to factor/factor#1474, it seems like triple quotes are not supported.

tancnle · 2021-03-15T11:00:25Z

@Ravlen What do you think about getting the regexes from Pygments?

diff --git lib/rouge/lexers/factor.rb lib/rouge/lexers/factor.rb
index 2a18f715..4a7458a5 100644
--- lib/rouge/lexers/factor.rb
+++ lib/rouge/lexers/factor.rb
@@ -243,8 +243,9 @@ module Rouge
         end
 
         # strings
-        rule %r/"""\s+.*?\s+"""/, Str
-        rule %r/"(\\.|[^\\])*?"/, Str
+        rule %r/"""\s(?:.|\n)*?\s"""/, Str
+        rule %r/"(?:\\\\|\\"|[^"])*"/, Str
+        rule %r/\S+"\s+(?:\\\\|\\"|[^"])*"/, Str
         rule %r/(CHAR:)(\s+)(\\[\\abfnrstv]*|\S)(?=\s)/, Str::Char
 
         # comments

dblessing · 2021-03-17T19:34:20Z

@tancnle I like this idea. They're vetted/tested.

Uses the same regex as pygments, and removes the triple quote regex, which isn't actually supported in factor

Ravlen · 2021-03-18T08:49:23Z

@tancnle @dblessing OK, updated with the regex from Pygments, but I dropped the """ string regex as I'm quite confident it isn't supported by Factor. See https://github.com/factor/factor/blob/master/core/strings/strings-tests.factor, and the comment Tan found at: factor/factor#1474 (comment)

tancnle · 2021-03-18T11:35:24Z

Travis CI seems to be problematic 🤔 @dblessing could you help to check what is going wrong with Travis integration?

dblessing · 2021-03-18T16:19:44Z

It looks like we've run out of build minutes due to Travis' new billing - https://blog.travis-ci.com/2020-11-02-travis-ci-new-billing.

We either need to apply for OSS allotment, or potentially use GitLab builds. What do you all think @tancnle @Ravlen. I think GitLab only offers 400 free minutes, but maybe they'd be willing to sponsor a 'plan' for Rouge?

tancnle · 2021-03-19T04:54:06Z

@dblessing Would it be much hassle to apply for an OSS allotment? It provides 10k credits which are build minutes I believe.

I think BuildKite or CircleCI might be a better choice given they have no build minute limit for OSS. I am keen to give you a hand on the CI migration if needed.

dblessing · 2021-03-22T19:11:22Z

@tancnle I've applied for credits. But it's not 10,000 automatic - I had to request the amount I think we need. I asked if they had visibility into historical usage. But AFAICT we use about 10 minutes per build - 1 minute per suite and we test 10 Ruby versions. Probably just 500-1000 minutes will suffice.

tancnle · 2021-03-23T00:20:14Z

@dblessing I think moving to BuildKite or CircleCI with unlimited build minute will remove the need to recalculate the allotment as we scale our pipeline. But it might require some work.

we test 10 Ruby versions

That's a lot. Are we thinking about deprecating old Ruby versions (I.e. no ongoing supported for MRI < 2.5)?

We can get those rolling on separate issues in the meanwhile, can't we?

dblessing · 2021-03-23T00:28:16Z

Is honestly prefer either to stay with Travis or move to GitLab. I agree we should consider removing some Ruby version support.

tancnle · 2021-03-23T01:37:33Z

No worries @dblessing. I am cool with staying with TravisCI and get this MR unblocked and merged. FYI I have created the issue to remove support for old Ruby version.

Ravlen · 2021-03-24T05:33:03Z

@dblessing @tancnle If we want to switch to GitLab, I'm reasonably confident I could write the pipeline configuration. There are 50k minutes available for open source projects, see https://about.gitlab.com/solutions/open-source/, if we want to look into it (though perhaps you'd need to move the whole project to GitLab to get the full 50k?)

dblessing · 2021-04-06T18:31:14Z

I still haven't heard back from Travis so I pinged them again on the support issue.

@Ravlen I think you're right - we'd need to either move the whole project or see if they would grant us a Premium license. I think the best case is Travis comes through for us, but if not we can see about GitLab CI.

tancnle · 2021-05-19T00:06:35Z

@dblessing Any news from Travis? If not, I think we can start migrating to another CI solution.

dblessing · 2021-05-19T00:17:54Z

@tancnle I thought I commented about 3 weeks ago saying we had CI allocation and asking @Ravlan to push again. Either a malfunction or I only dreamed it. Sorry all. It should be working now, though.

On Tue, May 18, 2021 at 7:06 PM Tan Le ***@***.***> wrote: @dblessing <https://github.com/dblessing> Any news from Travis? If not, I think we can start migrating to another CI solution. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1690 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AASIVFFMSNFZO6EJVGW3FJTTOL6J3ANCNFSM4Y5NLKIQ> .

-- Drew

Ravlen · 2021-05-20T09:48:25Z

@tancnle @dblessing Yay, finally the Travis build worked 😁

jneen · 2021-06-07T14:36:39Z

I for one would be 100% down to move to gitlab, so long as we could preserve the state of the issue tracker.

jneen · 2021-06-07T14:37:31Z

(And given that this software is used by gitlab I think it would be fitting :] )

Avoid catastrophic backtracing in regex

05dc42b

Ravlen commented Mar 11, 2021

View reviewed changes

Update factor lexer string regex

2b928fa

Uses the same regex as pygments, and removes the triple quote regex, which isn't actually supported in factor

dblessing mentioned this pull request Apr 6, 2021

Is Rouge unmaintained? #1700

Closed

Ravlen closed this May 20, 2021

Ravlen reopened this May 20, 2021

dblessing merged commit 78af25c into rouge-ruby:master Jun 7, 2021

Geod24 mentioned this pull request Aug 6, 2021

Replace Travis CI with Github workflow #1728

Merged

thewoolleyman mentioned this pull request Dec 10, 2021

Avoid backtracing in ceylon interpolation regex #1773

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Avoid catastrophic backtracing in regex #1690

Avoid catastrophic backtracing in regex #1690

Ravlen commented Mar 10, 2021

Ravlen Mar 11, 2021 •

edited

Loading

Ravlen Mar 11, 2021 •

edited

Loading

tancnle Mar 11, 2021 •

edited

Loading

tancnle Mar 15, 2021

tancnle commented Mar 15, 2021

dblessing commented Mar 17, 2021

Ravlen commented Mar 18, 2021 •

edited

Loading

tancnle commented Mar 18, 2021

dblessing commented Mar 18, 2021

tancnle commented Mar 19, 2021 •

edited

Loading

dblessing commented Mar 22, 2021

tancnle commented Mar 23, 2021 •

edited

Loading

dblessing commented Mar 23, 2021 via email •

edited

Loading

tancnle commented Mar 23, 2021

Ravlen commented Mar 24, 2021

dblessing commented Apr 6, 2021

tancnle commented May 19, 2021 •

edited

Loading

dblessing commented May 19, 2021 via email

Ravlen commented May 20, 2021

jneen commented Jun 7, 2021

jneen commented Jun 7, 2021

Avoid catastrophic backtracing in regex #1690

Avoid catastrophic backtracing in regex #1690

Conversation

Ravlen commented Mar 10, 2021

For the ghc_core lexer:

For the factor lexer:

Ravlen Mar 11, 2021 • edited Loading

Choose a reason for hiding this comment

Ravlen Mar 11, 2021 • edited Loading

Choose a reason for hiding this comment

tancnle Mar 11, 2021 • edited Loading

Choose a reason for hiding this comment

tancnle Mar 15, 2021

Choose a reason for hiding this comment

tancnle commented Mar 15, 2021

dblessing commented Mar 17, 2021

Ravlen commented Mar 18, 2021 • edited Loading

tancnle commented Mar 18, 2021

dblessing commented Mar 18, 2021

tancnle commented Mar 19, 2021 • edited Loading

dblessing commented Mar 22, 2021

tancnle commented Mar 23, 2021 • edited Loading

dblessing commented Mar 23, 2021 via email • edited Loading

tancnle commented Mar 23, 2021

Ravlen commented Mar 24, 2021

dblessing commented Apr 6, 2021

tancnle commented May 19, 2021 • edited Loading

dblessing commented May 19, 2021 via email

Ravlen commented May 20, 2021

jneen commented Jun 7, 2021

jneen commented Jun 7, 2021

For the `ghc_core` lexer:

For the `factor` lexer:

Ravlen Mar 11, 2021 •

edited

Loading

Ravlen Mar 11, 2021 •

edited

Loading

tancnle Mar 11, 2021 •

edited

Loading

Ravlen commented Mar 18, 2021 •

edited

Loading

tancnle commented Mar 19, 2021 •

edited

Loading

tancnle commented Mar 23, 2021 •

edited

Loading

dblessing commented Mar 23, 2021 via email •

edited

Loading

tancnle commented May 19, 2021 •

edited

Loading