-
Notifications
You must be signed in to change notification settings - Fork 13
chore: validate html descriptions, warn if invalid #196
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Stephen Rugh <rugh@adobe.com>
Signed-off-by: Stephen Rugh <rugh@adobe.com>
|
|
||
| Object.entries(fieldValidations).forEach(([field, validation]) => { | ||
| if (!validation.valid) { | ||
| context.logger.warn(`Validation failed for "${field}" field: ${validation.reason}`); |
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.
Is this log sufficient to inform the user? Hopefully users do not have to experience what I did - tracing backwards from malformed UX through EDS CDN, Azure, and finally realizing it is the data itself that is the problem.
@duynguyen let me know if there's a better way to alert users to bad data.
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 log message is good but it will potentially get lost among other logs.
IMO this should be tackled in a generic way, created #197
duynguyen
left a comment
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.
changes looking good, can we have tests for the validation cases?
|
@duynguyen I will write tests and let you know once I've pushed. |
|
Why don't we use a library to sanitize the html instead, allowing only the tags that edge delivery supports, or none if the meta descriptions becomes the |
Signed-off-by: Stephen Rugh <rugh@adobe.com>
…ests Signed-off-by: Stephen Rugh <rugh@adobe.com>
Closes #194
The reason this is happening is due to incorrect format in description data.
Technically this might happen for any user defined string which can be HTML.
This PR validates html for fields we are trying to template in that might be invalid.
DEBATE: