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

[MM-17616] v4.2: Update config.json file validation to prevent loosing config settings on upgrade #1015

Merged
merged 2 commits into from
Aug 8, 2019

Conversation

deanwhillier
Copy link
Contributor

@deanwhillier deanwhillier commented Aug 8, 2019

Summary

After analyzing the config.json file from a client experiencing the disappearing server problem after updating from 4.2.1 to 4.2.2, it seems that in their config.json files, the trayIconTheme option was set to a blank string instead of the light or dark options. The Validator was written to expect either light or dark and so the validation was failing. Digging deeper into the code, it looks like it might actually be possible to have a blank value for that option (it's really only a Linux option), so the Validator has been updated to allow for that.

While the team URL wasn't actually the problem in this case it is still a possibility, so the validator has also been updated to convert any \ characters to / characters and then further checks each updated URL individually to ensure they are each in fact a validly constructed URL. If any are not valid URLs they are individually filtered out, allowing the other URLs to remain instead of wiping out all configured servers for the sake of one.

In addition, the URL input in the Add Server modal now properly validates a URL provided by a user before saving to the config.json file.

These changes will need to be applied to the master branch as well as part of the pending security updates.

Issue link

https://mattermost.atlassian.net/browse/MM-17616

Test Cases

Test URL's:

  1. Install fresh copy of Desktop 4.2.1
  2. Add new server using https://community.mattermost.com\ (include the incorrect backslash)
  3. Verify that the new tab loads successfully
  4. Install Desktop 4.2.3 and launch
  5. Ensure that the server is still present and the URL has been corrected in the UI.

Test trayIconTheme option:

  1. Install fresh copy of Desktop 4.2.1
  2. Add any new valid server
  3. Verify that the new server loads successfully
  4. Exit the Desktop application completely
  5. Manually edit your config.json file to set the value for trayIconTheme to a blank string ("trayIconTheme": "")
  6. Install Desktop 4.2.3 and launch
  7. Ensure that the server is still present.

@deanwhillier deanwhillier added the 2: Dev Review Requires review by a core committer label Aug 8, 2019
@deanwhillier deanwhillier added the 3: QA Review Requires review by a QA tester label Aug 8, 2019
@amyblais amyblais mentioned this pull request Aug 8, 2019
@crspeller crspeller removed the 2: Dev Review Requires review by a core committer label Aug 8, 2019
@DSchalla DSchalla added 4: Reviews Complete All reviewers have approved the pull request and removed 3: QA Review Requires review by a QA tester labels Aug 8, 2019
@DSchalla DSchalla merged commit 44f01e2 into release-4.2 Aug 8, 2019
@DSchalla DSchalla deleted the MM-17616 branch August 8, 2019 19:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4: Reviews Complete All reviewers have approved the pull request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants