Repository: parse-community/Parse-SDK-JS
Pull request #1481 (2022-05-17 11:52:36)

fix: invalid name for `Parse.Role` throws incorrect error
### New Pull Request Checklist
<!--
    Check every following box [x] before submitting your PR.
    Click the "Preview" tab for better readability.
    Thanks for contributing to Parse Platform!
-->

- [x] I am not disclosing a [vulnerability](https://github.com/parse-community/parse-server/blob/master/SECURITY.md).
- [x] I am creating this PR in reference to an [issue](https://github.com/parse-community/Parse-SDK-JS/issues?q=is%3Aissue).

### Issue Description
<!-- Add a brief description of the issue this PR solves. -->

Creating a role with an invalid name does not throw an error, leaving the API to throw `Error: name is required`

Related issue: #1480

### Approach
<!-- Add a description of the approach in this PR. -->

Validate role name on create

### TODOs before merging
<!--
    Add TODOs that need to be completed before merging this PR.
    Delete TODOs that do not apply to this PR.
-->

- [x] Add tests

parse-github-assistant[bot]: 
### Thanks for opening this pull request!

- 🎉 We are excited about your hands-on contribution!

<!-- parse-issue-bot-meta-tag-id {"suggestPr":false,"thanksFeature":false,"thanksPr":true} -->


codecov[bot]: # [Codecov](https://codecov.io/gh/parse-community/Parse-SDK-JS/pull/1481?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=parse-community) Report
> Merging [#1481](https://codecov.io/gh/parse-community/Parse-SDK-JS/pull/1481?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=parse-community) (674614d) into [alpha](https://codecov.io/gh/parse-community/Parse-SDK-JS/commit/3d3970855e36a83c9e56b64e3628d283f584a386?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=parse-community) (3d39708) will **increase** coverage by `0.00%`.
> The diff coverage is `100.00%`.

```diff
@@           Coverage Diff           @@
##            alpha    #1481   +/-   ##
=======================================
  Coverage   99.94%   99.94%           
=======================================
  Files          61       61           
  Lines        5949     5954    +5     
  Branches     1357     1357           
=======================================
+ Hits         5946     5951    +5     
  Misses          3        3           
```


| [Impacted Files](https://codecov.io/gh/parse-community/Parse-SDK-JS/pull/1481?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=parse-community) | Coverage Δ | |
|---|---|---|
| [src/ParseRole.js](https://codecov.io/gh/parse-community/Parse-SDK-JS/pull/1481/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=parse-community#diff-c3JjL1BhcnNlUm9sZS5qcw==) | `100.00% <100.00%> (ø)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/parse-community/Parse-SDK-JS/pull/1481?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=parse-community).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=parse-community)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/parse-community/Parse-SDK-JS/pull/1481?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=parse-community). Last update [3d39708...674614d](https://codecov.io/gh/parse-community/Parse-SDK-JS/pull/1481?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=parse-community). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=parse-community).

dblythy: Yep!

Patch:
diff --git a/src/ParseRole.js b/src/ParseRole.js
index 07d4c8f..c505604 100644
--- a/src/ParseRole.js
+++ b/src/ParseRole.js
@@ -75,6 +75,7 @@ class ParseRole extends ParseObject {
    * @returns {(ParseObject|boolean)} true if the set succeeded.
    */
   setName(name: string, options?: mixed): ParseObject | boolean {
+    this._validateName(name);
     return this.set('name', name, options);
   }
 
@@ -108,6 +109,18 @@ class ParseRole extends ParseObject {
     return this.relation('roles');
   }
 
+  _validateName(newName) {
+    if (typeof newName !== 'string') {
+      throw new ParseError(ParseError.OTHER_CAUSE, "A role's name must be a String.");
+    }
+    if (!/^[0-9a-zA-Z\-_ ]+$/.test(newName)) {
+      throw new ParseError(
+        ParseError.OTHER_CAUSE,
+        "A role's name can be only contain alphanumeric characters, _, " + '-, and spaces.'
+      );
+    }
+  }
+
   validate(attrs: AttributeMap, options?: mixed): ParseError | boolean {
     const isInvalid = super.validate(attrs, options);
     if (isInvalid) {
@@ -125,14 +138,10 @@ class ParseRole extends ParseObject {
           "A role's name can only be set before it has been saved."
         );
       }
-      if (typeof newName !== 'string') {
-        return new ParseError(ParseError.OTHER_CAUSE, "A role's name must be a String.");
-      }
-      if (!/^[0-9a-zA-Z\-_ ]+$/.test(newName)) {
-        return new ParseError(
-          ParseError.OTHER_CAUSE,
-          "A role's name can be only contain alphanumeric characters, _, " + '-, and spaces.'
-        );
+      try {
+        this._validateName(newName);
+      } catch (e) {
+        return e;
       }
     }
     return false;


Test patch:
diff --git a/integration/test/ParseACLTest.js b/integration/test/ParseACLTest.js
index 75b7d31..4251f05 100644
--- a/integration/test/ParseACLTest.js
+++ b/integration/test/ParseACLTest.js
@@ -533,4 +533,13 @@ describe('Parse.ACL', () => {
     const obj1withInclude = await query.first();
     assert(obj1withInclude.get('other').get('ACL'));
   });
+
+  it('prevents save with invalid role name', async () => {
+    expect(() => new Parse.Role(':%#', new Parse.ACL())).toThrow(
+      new Parse.Error(
+        Parse.Error.OTHER_CAUSE,
+        `A role's name can be only contain alphanumeric characters, _, -, and spaces.`
+      )
+    );
+  });
 });
diff --git a/src/__tests__/ParseRole-test.js b/src/__tests__/ParseRole-test.js
index e2e7fa0..825ec60 100644
--- a/src/__tests__/ParseRole-test.js
+++ b/src/__tests__/ParseRole-test.js
@@ -45,6 +45,15 @@ describe('ParseRole', () => {
     expect(role.getName()).toBe('');
   });
 
+  it('should throw error string with invalid name', () => {
+    expect(() => new ParseRole('invalid:name', new ParseACL())).toThrow(
+      new ParseError(
+        ParseError.OTHER_CAUSE,
+        "A role's name can be only contain alphanumeric characters, _, " + '-, and spaces.'
+      )
+    );
+  });
+
   it('can validate attributes', () => {
     const acl = new ParseACL({ aUserId: { read: true, write: true } });
     const role = new ParseRole('admin', acl);