-
Notifications
You must be signed in to change notification settings - Fork 174
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
[Instruments] Escaping issue sub-optimal - Redirect on save success to load defaults from database #7776
Conversation
@driusan on second thought, I don't see why this would need a changelog update... the other fix does, not this one |
'&candID=' . urlencode($candID) . | ||
'&sessionID=' . urlencode($sessionID); | ||
return (new \LORIS\Http\Response()) | ||
->withHeader("Location", $url); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The response should also have a 303 See Other response code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what other response code ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No other one. The code's name is See Other
.
https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/303
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
line 2846 above in the same file has the exact same code, should we move this to a function ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2846 already had the 303? I don't see why it would be moved to a function.. setting the response code is always going to be context specific
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dont like code duplication
3990912
to
8613bc0
Compare
@driusan urs again |
@CamilleBeau please add passed manual test |
Brief summary of changes
This PR fixes the escaping issue that occurs when a text field in an instrument contains an HTML special character. The solution employed here is to redirect on successful save to reload data directly from the database instead of loading it using the _POST data.
Cons:
Pros:
Alternate to: #7777
Fixes #7489
Replaces #7490
Testing instructions (if applicable)
Make sure to test other usecases as well
"
,<
,>
or&
in the stringLink(s) to related issue(s)