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

Cors origin RegExp issues #292

Closed
adrien2p opened this issue Nov 15, 2022 · 10 comments
Closed

Cors origin RegExp issues #292

adrien2p opened this issue Nov 15, 2022 · 10 comments

Comments

@adrien2p
Copy link

adrien2p commented Nov 15, 2022

Hey there, first of all thank you for this middleware.

I am having some issues using the regexp with the origin using the cors middleware.
When I set the value to be /http:\\/\\/localhost:[0-9]+$/ the regexp is properly constructed. Putting some breakpoints in the lib I can see that the origin is allowed using the regexp

Screenshot 2022-11-15 at 11 26 40

But for un unknown reason after the header are set properly, I still get a cors issue

Screenshot 2022-11-15 at 11 27 01

Funny enough, the cors error happen only on the auth route as you can see here

Screenshot 2022-11-15 at 11 31 07

The cors are setup on app using app.use and then later on the auth routes are added to a Router() and the router is consumed by app.use

Would you have any idea what is happening?

@adrien2p
Copy link
Author

I am not sure if that pr could fix the issue but I will take a look at test that locally to see #276

@adrien2p
Copy link
Author

@dougwilson any thoughts on that behaviour?

@dougwilson
Copy link
Contributor

Hello, and sorry you are having trouble. From reading your issue, it sounds like you debugged through and saw that your regexp matched and the module set the expected response headers, but yet cors still failed, is that correct? If so, that sounds strange, and I'm not sure what to make of it, exactly. I can help debug your issue too, but I need you to provide a minimal reproducable example that I can run and get the same issue so I can debug it. You're always welcome to submit a patch directly as well.

@adrien2p
Copy link
Author

Thank you for your answer, yes this is exactly the flow. The thing is that I tried to check everywhere in the middleware to see if something went wrong and did not find anything. I ve linked the pr i ve made in our repo but the best way to reproduce it might be to just test to use a regexp such as /http:\/\/localhost:700\d/ or similar and pass it to to the origin option and see if you obtain the same result as me ☺️ wdyt?

@dougwilson
Copy link
Contributor

test to use a regexp such as /http://localhost:700\d/ or similar and pass it to to the origin option and see if you obtain the same result as me ☺️ wdyt?

Yea, I tested the regexp locally and there are tests in our test suite as well that are working fine. So something different between the environments and code or something and not sure how to determine that since what i tried before responding works and the test suite verifies thay regexp matching works. I believe absolutely you are having an issue, but just don't know the cause and how to reproduce it to debug, which is why i asked for one if you're interested in having me debug :)

@adrien2p
Copy link
Author

adrien2p commented Nov 16, 2022

Thank you for your feedback, I might have a lead. So while debugging it I though was not to call the configureOrigin(options, req) multiple times and see the output, while doing that it sometimes return the origins url and sometimes it return false and that happen randomly

here are the SS during the debug just using my ide evaluator to execute it multiple times. It looks pretty weird as a behaviour
Screenshot 2022-11-16 at 22 34 38

Screenshot 2022-11-16 at 22 32 02

My question is the next one, I ve seen a pr around that, could it be the same scenario? in that case it does not require heavy load to find out as only few re evaluation and we get both result false/localhost. And that would explain why some requests does not get a cors error and other get it

@adrien2p
Copy link
Author

adrien2p commented Nov 16, 2022

another info is that evaluating allowedOrigin.test(origin) give me true/false each time 1: true, 2: false, etc but this might be link to the global flag as after a first test the last index change and therefore the following test fail

@adrien2p
Copy link
Author

adrien2p commented Nov 16, 2022

Indeed, the issue is link to the usage of flag which can give random behaviour to the mutation of the underlying properties that RegExp use under the hood. Just done another test and now it is working.

@dougwilson
Copy link
Contributor

dougwilson commented Nov 16, 2022

Which flag are you referring to? Neither regexp you posted /http:\\/\\/localhost:[0-9]+$/ and /http:\/\/localhost:700\d/ have any flags set.

@adrien2p
Copy link
Author

adrien2p commented Nov 16, 2022

Sorry missed it but I was using /g flag among other tests and it was certainly the culprit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants