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
Strip HTML tags in API error messages #14213
Conversation
Tested locally and this would decode messages for exceptions that also don't call CC @tsteur since it would be a change to the API |
I don't think it's a good idea to decode all errors as it could definitely lead to various security issues. If there was such a Took me a while to think about it... I would say in general, an API shouldn't return an HTML message as you don't know if the consumer can process HTML. Often it would not be possible to show HTML, eg in a mobile app etc. I would say we strip HTML tags server side for these messages, and / or have two properties: I think in the mentioned case in #12300 we could maybe just keep things simple and strip the tags and show it without the html tags? |
Sounds good to me if it works this way 👍 |
stripping the tags should work as well. I'll adjust the PR |
a860807
to
5d519ae
Compare
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.
There are some test failures, but looks good
Random thought: Should this maybe only happen when the ROOT request is an API request? This way a lot of code can maybe still format a message correctly with HTML? |
Haven't tested (of course would be good to), this code might be skipped if the original format is used. |
5d519ae
to
6e23b8a
Compare
I've added the check for root api requests... |
looks good |
fixes #12300