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
[WebProfilerBundle] Fix regexp #14217
Conversation
@romqin thanks a lot for fixing this annoying issue! |
👍 indeed I wonder how to make it work with absolute URL ? |
@aitboudad what if I add this : + var path = url.substr({{ app.request.basePath|length }});
- if (url.substr(0, 1) === '/' && !url.match(new RegExp({{ excluded_ajax_paths|json_encode|raw }}))) {
+ if (path.substr(0, 1) === '/' && !path.match(new RegExp({{ excluded_ajax_paths|json_encode|raw }}))) { |
- var path = url.substr({{ app.request.basePath|length }});
+ var path = url.replace('{{ app.request.schemeAndHttpHost }}', ''); |
Or this, but maybe it's overkill : var path = url;
if (url.substr(0, 1) === '/') {
if (0 === url.indexOf('{{ app.request.getBasePath }}')) {
path = url.substr({{ app.request.basePath|length }});
}
}
else if (0 === url.indexOf('{{ app.request.getSchemeAndHttpHost() ~ app.request.getBasePath }}')) {
path = url.substr({{ (app.request.getSchemeAndHttpHost() ~ app.request.basePath)|length }});
}
if (!path.match(new RegExp({{ excluded_ajax_paths|json_encode|raw }}))) { |
look good but I'd prefer to use var path = (url.substr(0, 1) === '/') ? url.replace('{{ app.request.basePath }}', ''):url.replace('{{ app.request.schemeAndHttpHost ~ app.request.basePath }}', '');
if (path.substr(0, 1) === '/' && !path.match(new RegExp({{ excluded_ajax_paths|json_encode|raw }}))) { |
👍 |
@aitboudad the issue with |
@stof no it will replace only the first occurrence. |
@aitboudad but what if the path is not found at the beginning, but is found later in the string ? |
@stof got it - ? url.replace('{{ app.request.basePath }}', '')
- : url.replace('{{ app.request.schemeAndHttpHost ~ app.request.basePath }}', '');
+ ? url.replace(/^({{ app.request.basePath|e('js') }})/, '')
+ : url.replace(/^({{ (app.request.schemeAndHttpHost ~ app.request.basePath)|e('js') }})/, ''); is that ok ? |
@aitboudad no. you need to escape the path for a usage into a regex (especially given that |
@romqin this looks like the right solution (except the escaping needed to output the value as a JS string) |
Like this ? var path = url;
if (url.substr(0, 1) === '/') {
if (0 === url.indexOf('{{ app.request.basePath|e('js') }}')) {
path = url.substr({{ app.request.basePath|length }});
}
}
else if (0 === url.indexOf('{{ (app.request.schemeAndHttpHost ~ app.request.basePath)|e('js') }}')) {
path = url.substr({{ (app.request.schemeAndHttpHost ~ app.request.basePath)|length }});
}
if (!path.match(new RegExp({{ excluded_ajax_paths|json_encode|raw }}))) { |
@romqin now it work for all url, you need check if (path.substr(0, 1) === '/' && !path.match(new RegExp({{ excluded_ajax_paths|json_encode|raw }}))) { |
Would be great to have this fix in 2.7 beta1 |
Thank you @romqin. |
This PR was squashed before being merged into the 2.6 branch (closes #14217). Discussion ---------- [WebProfilerBundle] Fix regexp | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #12122 | License | MIT | Doc PR | Old PR #12124 Commits ------- 091b37e [WebProfilerBundle] Fix regexp
@fabpot this PR broke Silex-WebProfiler ~2.0@dev, see silexphp/Silex-WebProfiler#64 |
We should not use the |
…tes (stof) This PR was merged into the 2.6 branch. Discussion ---------- Avoid using the app global variable in the profiler templates | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | small one for people overwriting the WebDebugToolbarListener methods | Deprecations? | no | Tests pass? | yes | Fixed tickets | silexphp/Silex-WebProfiler#64 | License | MIT | Doc PR | n/a #14217 introduced a usage of the ``app`` global variables in profiler templates, while we previously removed all such usages to avoid the dependency on TwigBundle in these templates. This keeps them usable outside the fullstack framework (for instance in Silex). I had to do a small BC break to be able to pass the request in the place where we were not yet injecting the Request in the template. Commits ------- 415c79e Avoid using the app global variable in the profiler templates
This PR was squashed before being merged into the 2.6 branch (closes symfony#14217). Discussion ---------- [WebProfilerBundle] Fix regexp | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | symfony#12122 | License | MIT | Doc PR | Old PR symfony#12124 Commits ------- 091b37e [WebProfilerBundle] Fix regexp
Old PR #12124