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

Fixes #37582 - use textarea in host comment edit #10218

Merged
merged 1 commit into from
Jul 23, 2024

Conversation

MariaAga
Copy link
Member

Had to change the css since for long inputs the comment text would go out of the card border.
Added set default on submit so if a user changes the comment, saves, and then changes it again but clicks the cancel it wont go to the value that the page loaded

@MariaAga
Copy link
Member Author

added white-space: pre-line; so that the comment view will show newlines

@sbernhard
Copy link
Contributor

Nice. Thank you.
Is there a validation / check somewhere to prevent XSS?

@MariaAga
Copy link
Member Author

By default, React DOM escapes any values embedded in JSX before rendering them. (https://legacy.reactjs.org/docs/introducing-jsx.html#jsx-prevents-injection-attacks)
Is there other risk I didnt see?

@sbernhard
Copy link
Contributor

By default, React DOM escapes any values embedded in JSX before rendering them. (https://legacy.reactjs.org/docs/introducing-jsx.html#jsx-prevents-injection-attacks) Is there other risk I didnt see?

Oh, this is great. Thank you very much.

Do we somewhere have a XSS tests which tries to inject certain JS stuff in textarea / other user-defined input fields and then tries to find out if the JS would be escaped / not espaced?

@nadjaheitmann
Copy link
Contributor

I tested this and what happens is:

  1. If I enter a comment value and submit, that is fine.
  2. When I edit the comment and submit, that is also fine.
  3. When I edit the comment a third time and then click abort, then the first value that I initially added in the current session re-appears instead the one I added in 2)

@MariaAga
Copy link
Member Author

Do we somewhere have a XSS tests which tries to inject certain JS stuff in textarea / other user-defined input fields and then tries to find out if the JS would be escaped / not espaced?

Not in our tests in the repo, since we use textarea from react/rails which should be secure for that.

When I edit the comment a third time and then click abort, then the first value that I initially added in the current session re-appears instead the one I added in 2)

Thanks, had a typo in the function call, should be good now

@nadjaheitmann
Copy link
Contributor

Thanks, had a typo in the function call, should be good now

Works now, thanks!

@MariaAga MariaAga requested a review from jeremylenz July 23, 2024 08:11
Copy link
Contributor

@jeremylenz jeremylenz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code LGTM, approving based on @nadjaheitmann's testing. thanks @MariaAga!

@jeremylenz jeremylenz merged commit 2d5bd87 into theforeman:develop Jul 23, 2024
66 of 67 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
4 participants