Skip to content

Improve JS course key validation to not allow special chars. - #6637

Merged
antoviaque merged 1 commit into
openedx:masterfrom
open-craft:mtyaka/validate-chars
Jan 20, 2015
Merged

Improve JS course key validation to not allow special chars.#6637
antoviaque merged 1 commit into
openedx:masterfrom
open-craft:mtyaka/validate-chars

Conversation

@mtyaka

@mtyaka mtyaka commented Jan 15, 2015

Copy link
Copy Markdown
Contributor

Background: Existing JS validation on new course and library forms prevents the user from submitting course/library keys that contain special characters, but it fails to detect these special characters: !'()*.
Discussions: The issue was discovered by @catong while reviewing open-craft#23 (diff)
Affected Components: Studio
Jira Ticket: https://openedx.atlassian.net/browse/SOL-233
Test Instructions: Go to the Studio home page and click the 'New Course' button to display the new course form. Put a string containing one of the special !'()* characters in any of 'Organization', 'Course Number', or 'Course Run' fields. Without this patch, the JS validation will fail to detect the special characters and will let you submit the form. Course creation will fail on the server because special characters are not allowed in these fields. With this patch enabled, JS validation will detect the special characters and will not let you submit the form (see screenshot).
Internal Code Review PR: open-craft#28
Partner Information: 3rd party-hosted open edX instance, for an edX solutions client.

screen shot 2015-01-09 at 08 50 28

Course and Library keys cannot contiain !'()* special characters,
but the JS validation on the new course/library failed to detect
these characters.

`encodeURIComponent` is used to check the string for special characters,
but `encodeURIComponent` does not encode these characters: -_!~*'().
(see https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/encodeURIComponent)

The -_~ characters are allowed in course keys, but !'()* are not,
so add an explicit check for these characters to make sure a field
containing these characters does not pass the validation.
@openedx-webhooks

Copy link
Copy Markdown

Thanks for the pull request, @mtyaka! It looks like you're a member of a company that does contract work for edX. If you're doing this work as part of a paid contract with edX, you should talk to edX about who will review this pull request. If this work is not part of a paid contract with edX, then you should ensure that there is an OSPR issue to track this work in JIRA, so that we don't lose track of your pull request.

To automatically create an OSPR issue for this pull request, just visit this link: http://openedx-webhooks.herokuapp.com/github/process_pr?number=6637&repo=edx%2Fedx-platform

@Kelketek

Copy link
Copy Markdown
Contributor

👍

@mtyaka

mtyaka commented Jan 16, 2015

Copy link
Copy Markdown
Contributor Author

@cahrens @dmitchell Can you review this, please?

@openedx-webhooks

Copy link
Copy Markdown

Thanks for the pull request, @mtyaka! It looks like you're a member of a company that does contract work for edX. If you're doing this work as part of a paid contract with edX, you should talk to edX about who will review this pull request. If this work is not part of a paid contract with edX, then you should ensure that there is an OSPR issue to track this work in JIRA, so that we don't lose track of your pull request.

To automatically create an OSPR issue for this pull request, just visit this link: http://openedx-webhooks.herokuapp.com/github/process_pr?number=6637&repo=edx%2Fedx-platform

@cahrens

cahrens commented Jan 20, 2015

Copy link
Copy Markdown

👍

@cahrens

cahrens commented Jan 20, 2015

Copy link
Copy Markdown

BTW, @dmitchell is no longer at edX. I think this is OK to merge without another review.

antoviaque added a commit that referenced this pull request Jan 20, 2015
Improve JS course key validation to not allow special chars.
@antoviaque
antoviaque merged commit 0c7620a into openedx:master Jan 20, 2015
@mtyaka
mtyaka deleted the mtyaka/validate-chars branch January 8, 2016 08:01
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.

5 participants