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

[BUG] regex group variables reset on subroutine transitions #297

Open
2 tasks done
richardmarshall opened this issue Apr 5, 2024 · 2 comments
Open
2 tasks done
Assignees
Labels
bug Something isn't working

Comments

@richardmarshall
Copy link
Collaborator

Kind of proposals

  • Simulator
  • Testing

Describe the problem

When changing scope to a new subroutine via call or a function expression the interpreter resets the re.group.# variables in the same way it does for local variables. This is not correctly following the behavior of Fastly.

Additionally accessing a re group variable before performing a regex match currently produces an undefined variable interpreter error instead of returning a NotSet value.

VCL code that cause the problem / reproduceable

sub foo {
  set req.http.foo1 = re.group.1;
  if (req.http.test ~ "([^.]+)") {}
  set req.http.foo2 = re.group.1;
}

sub bar {
  set req.http.bar1 = re.group.1;
  if (req.http.test ~ "(.*)") {}
  set req.http.bar2 = re.group.1;
  call foo;
  set req.http.bar3 = re.group.1;
}
  ●  [RECV] test

    Assertion Error: Assertion error: expect=aoeu.aoeu, actual=
    Actual Value: 

    5|   assert.equal(req.http.bar2, "aoeu.aoeu");
    6|   assert.equal(req.http.foo1, "aoeu.aoeu");
    7|   assert.equal(req.http.foo2, "aoeu");

Expected behavior

https://fiddle.fastly.dev/fiddle/a3a1975b

@ysugimoto
Copy link
Owner

Oh, I didin't know this spec...

I think we should follow the behavior in the interpreter but the linter should report an error as WARNING because this spec causes a potential bug.

@richardmarshall
Copy link
Collaborator Author

@ysugimoto A linter warning is a good idea. I updated #294 to include a fix for the interpreter since I was already moving around the subroutine state changes. Can make another PR that adds warning logic to the linter.

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
Development

No branches or pull requests

2 participants