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

URLs are often double-escaped in smarty templates #4569

Closed
NateWr opened this issue Mar 4, 2019 · 3 comments
Closed

URLs are often double-escaped in smarty templates #4569

NateWr opened this issue Mar 4, 2019 · 3 comments
Assignees
Labels
Housekeeping:1:Todo Any dependency management or refactor that would be nice to have some day. Try Me This issue might be good for a new contributor. Can you help us?
Milestone

Comments

@NateWr
Copy link
Contributor

NateWr commented Mar 4, 2019

We often use {url|escape ...} in a Smarty template when the URL contains user input. However, the {url} function already escapes the final URL, so this is unnecessary.

All instances of {url|escape ...} should be changed to {url ...}.

@NateWr NateWr added the Try Me This issue might be good for a new contributor. Can you help us? label Mar 4, 2019
@asmecher
Copy link
Member

asmecher commented Mar 4, 2019

There is one case where the form {url|escape ...} is used in Javascript -- that should be changed to use json_encode instead. See lib/pkp/templates/controllers/grid/queries/form/queryForm.tpl:

$('#queryForm').pkpHandler(
 '$.pkp.controllers.form.CancelActionAjaxFormHandler',
   {ldelim}
     cancelUrl: {if $isNew}'{url|escape:javascript op="deleteQuery" queryId=$queryId csrfToken=$csrfToken params=$actionArgs escape=false}'{else}null{/if}
   {rdelim}
 );

The cancelUrl line should instead read:

 cancelUrl: {if $isNew}{url|json_encode op="deleteQuery" queryId=$queryId csrfToken=$csrfToken params=$actionArgs escape=false}{else}null{/if}

NateWr added a commit to NateWr/pkp-lib that referenced this issue Sep 5, 2022
NateWr added a commit to NateWr/ojs that referenced this issue Sep 5, 2022
NateWr added a commit to NateWr/omp that referenced this issue Sep 5, 2022
NateWr added a commit to NateWr/ops that referenced this issue Sep 5, 2022
@NateWr
Copy link
Contributor Author

NateWr commented Sep 5, 2022

@NateWr NateWr assigned NateWr and unassigned thinkbulecount2 Sep 5, 2022
@NateWr NateWr added the Housekeeping:1:Todo Any dependency management or refactor that would be nice to have some day. label Sep 5, 2022
@NateWr NateWr added this to the 3.4 milestone Sep 5, 2022
NateWr added a commit that referenced this issue Sep 6, 2022
#4569 Use json_encode for URLs passed to JavaScript
NateWr added a commit to pkp/omp that referenced this issue Sep 6, 2022
NateWr added a commit to pkp/ops that referenced this issue Sep 6, 2022
NateWr added a commit to pkp/ojs that referenced this issue Sep 6, 2022
@NateWr
Copy link
Contributor Author

NateWr commented Sep 6, 2022

Merged to main.

@NateWr NateWr closed this as completed Sep 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Housekeeping:1:Todo Any dependency management or refactor that would be nice to have some day. Try Me This issue might be good for a new contributor. Can you help us?
Projects
None yet
Development

No branches or pull requests

3 participants