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

Escape markdown characters in user input strings #171

Closed
anku255 opened this issue Nov 1, 2019 · 10 comments · Fixed by #183
Closed

Escape markdown characters in user input strings #171

anku255 opened this issue Nov 1, 2019 · 10 comments · Fixed by #183
Assignees

Comments

@anku255
Copy link
Contributor

anku255 commented Nov 1, 2019

Describe the bug
Sometimes the username can itself contain markdown characters and it would render improperly. We need to escape markdown character for it to actually render the correct username.

For example:

* Twitter: [@__anku__](https://twitter.com/__anku__)

renders as

expected output

To Reproduce
Use markdown characters while entering username/email.

I am not exactly sure how to solve this but I think we need to create a function that will escape all markdown characters and use that function to sanitize user input.

I would like to take a shot at this :)

@kefranabg
Copy link
Owner

Nice catch ! Thanks @anku255

@anku255
Copy link
Contributor Author

anku255 commented Nov 11, 2019

Hi! @kefranabg

I am thinking of escaping markdown characters in clean-context.js file. But I am not sure about the values that should be escaped. I am thinking of two ways to do this -

  1. Escape only Github, Twitter, LinkedIn, and Patreon. The problem with this approach is we need to update this list every time we add a new value that needs to be escaped.

  2. We can escape every key in the context but the problem with this is that there some keys having boolean values, some are an array of objects like isGithubRepos and projectPrerequisites. Also, I am not sure if passing every key through markdown-escape would be a good idea.

@kefranabg
Copy link
Owner

Let's just escape Github, Twitter, LinkedIn, and Patreon for now.

The problem with this approach is we need to update this list every time we add a new value that needs to be escaped.

Not sure to get it. Could you provide an example?

@anku255
Copy link
Contributor Author

anku255 commented Nov 12, 2019

Let's just escape Github, Twitter, LinkedIn, and Patreon for now.

The problem with this approach is we need to update this list every time we add a new value that needs to be escaped.

Not sure to get it. Could you provide an example?

Let's say we also want to have Codepen or Dribble username then we would need to add them to that list as well. It's unlikely we will add many sites but if we do, we need to remember to escape markdown.

@kefranabg
Copy link
Owner

Instead of doing a list, maybe we could use the escape markdown function inside each social network object prompt, with the transform property?

If we consider that when we want to add a new social network function, we take a look at existing ones, it would be harder to forget the escape markdown what do you think?

@anku255
Copy link
Contributor Author

anku255 commented Nov 13, 2019

Instead of doing a list, maybe we could use the escape markdown function inside each social network object prompt, with the transform property?

If we consider that when we want to add a new social network function, we take a look at existing ones, it would be harder to forget the escape markdown what do you think?

Do you mean the transformer function of inquirer.js?

I looked into the documentation and it says -

transformer: (Function) Receive the user input, answers hash and option flags, and return a transformed value to display to the user. The transformation only impacts what is shown while editing. It does not modify the answers hash.

It doesn't change the answer. I tried it and only the output shown to the user is transformed but the actual answer value is not. Take a look at the screen record below:

Screen Recording 2019-11-13 at 11 48 30 AM

@kefranabg
Copy link
Owner

Oops, I mean the filter function from inquirer 😉

@kefranabg
Copy link
Owner

The is an example in author-linkedin.js for example.

@anku255
Copy link
Contributor Author

anku255 commented Nov 13, 2019

Oops, I mean the filter function from inquirer 😉

Oh yeah! I forgot about that one 😄

Okay, how should I go about this? There is already a function cleanSocialNetworkUsername. Should I escape the markdown there itself as shown below:

// utils.js
const escape = require('markdown-escape');

const cleanSocialNetworkUsername = input => escape(input.replace(/^@/, ''))

OR

inside the question -

// author-linkedin.js
const escape = require('markdown-escape');
const { cleanSocialNetworkUsername } = require('../utils')

module.exports = () => ({
  type: 'input',
  message: '💼  LinkedIn username (use empty value to skip)',
  name: 'authorLinkedInUsername',
  filter: input => escape(cleanSocialNetworkUsername(input))
})

I prefer the first approach because of the DRY principle. What do you think?

@kefranabg
Copy link
Owner

Inside the cleanSocialNetworkUsername function seems better ;)

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 a pull request may close this issue.

2 participants