Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

NON_ALPHANUMERIC_REGEXP in ngSanitize #3192

Closed
JasonCust opened this issue Jul 10, 2013 · 1 comment · Fixed by #3206
Closed

NON_ALPHANUMERIC_REGEXP in ngSanitize #3192

JasonCust opened this issue Jul 10, 2013 · 1 comment · Fixed by #3206

Comments

@JasonCust
Copy link

Within the ngSanitize module, the stated purpose of the regex NON_ALPHANUMERIC_REGEXP is 'Match everything outside of normal chars and " (quote character)', which it appears to do.

However I have two questions regarding this:

  1. Why are escaped formatting characters like newline (\n), tab (\t), etc. included in this?
  2. Why is a capture used in the regex when it's never used? Also, couldn't it be simplified from: /([^\#-~| |!])/g to: /[^ -~]/g

I think the simplified character class reads a bit more clear. Also, it appears the intended use of the pipes in the original character class are for an 'or' which is not necessary as character classes by definition match character by character. In this case they are simply just a pipe character. Escaping the pound symbol is also not necessary.

If it was just an oversight on the escaped formatting characters mentioned above, the simplified regex could then be updated to: /[^\n\t -~]/g.

I apologize if I have glossed over something in the source.

@Narretz
Copy link
Contributor

Narretz commented Jul 2, 2014

@JasonCust you can send another pull request with these changes, since they are gone from your master branch I didn't actually merge the PR you send; there is a strange bug with Github. It shows as merged, but I just reopened the issue.
I reopened the PR, and your master branch was synced with angular master, and as there were no different commits, it was automatically merged.

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

Successfully merging a pull request may close this issue.

2 participants