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

bug : fixed response formatting issue using marked library #102

Conversation

VishalPawar1010
Copy link
Contributor

Description

This PR fixes #62 Copilot Response Formatting Issue

  • Resolved response formatting issue using marked library

@VishalPawar1010 VishalPawar1010 changed the title #62 : resolved response formatting issue using marked library bug : fixed response formatting issue using marked library May 13, 2024
@VishalPawar1010
Copy link
Contributor Author

@shivay-at-pieces Could you please review the PR. Thanks

@@ -180,7 +180,7 @@ export function CopilotChat(): React.JSX.Element {
</div>
<div className="messages">
<div>
<p>{message}</p>
<p dangerouslySetInnerHTML={{ __html: marked(message) }}></p>
Copy link
Contributor

Choose a reason for hiding this comment

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

Hey @VishalPawar1010 I would recommend to avoid using dangerouslySetInnerHTML in React as it can be risky because it can expose your application to Cross-Site Scripting (XSS) attacks if not handled properly.

Can you Sanitize the HTML output that you render from the marked library. Or you can use remark or rehype libraries

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure @shivay-at-pieces , will look into suggested options to ensure regarding Cross-Site Scripting (XSS) attacks

  1. To sanitise the HTML output that render from the marked library
  2. Use alternative libraries like remark or rehype

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey @shivay-at-pieces ,

I have proposed 2 (solutions) PRs for this issue #62 .

  1. Updated current PR with addition of sanitization to marked library and removal of dangerouslySetInnerHTML
  2. Resolved response formatting using ReactMarkdown and rehype library : bug : fixed response formatting using rehype and ReactMarkdown #111

Please let me know for any.

Copy link
Contributor

Choose a reason for hiding this comment

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

Similar problem like in the #111 the type definitions are missing. Please add those

Copy link
Contributor Author

@VishalPawar1010 VishalPawar1010 Jun 2, 2024

Choose a reason for hiding this comment

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

I have added type definitions as required in both PRs. Please have a look and let me know for any.

@shivay-at-pieces
Copy link
Contributor

Closing this since we have #111 open

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Copilot Response Formatting Issue
2 participants