-
Notifications
You must be signed in to change notification settings - Fork 787
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
Add test suite for Jsonp middleware #6958
base: series/0.23
Are you sure you want to change the base?
Conversation
Codecov Report
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more @@ Coverage Diff @@
## series/0.23 #6958 +/- ##
===============================================
+ Coverage 71.22% 71.42% +0.20%
===============================================
Files 340 340
Lines 10362 10362
Branches 886 886
===============================================
+ Hits 7380 7401 +21
+ Misses 2982 2961 -21
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Still failing on native
|
Ah, regex might not be working properly on Native. It happens sometmes. |
Are these look-ahead/behinds? Those are not supported on Native IIRC. |
Regex was changed last here: So it's perhaps been broken for a while. |
I restored parity with the original version, because the original author is a thoughtful type and it was probably right. The original changed because Unicode escape sequences are deprecated in triple quotes. Something went wrong in the translation, so I took an alternate approach of going to single quotes. It never worked on Native and without investment never will. I agree with the deprecation strategy in #6959, because even if it worked splendidly, Jsonp is a horrible idea in 2023. Curiosity got the best of me, and after @valencik went to all the work of testing it, we might as well leave behind something that works. |
Also: I'm surprised we're not validating this with a macro. But I don't see any other production, literal regexes in the code, other than |
Wow, amazing work, folks! |
This PR add some tests for the Jsonp middleware.
Unfortunately, in doing so I encountered regex exceptions on the callback validation regex.
These new tests pass if I simplify the regex to remove most of the character ranges.