Validate the Preferences when fetching them from storage #9936
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…emove the `BasePreferences.reload` method Given that the various Preferences are currently, and have been for quite some time, only used when initializing `PDFViewerApplication` re-loading them when a new PDF file is opened in the viewer is essentially a no-op. Furthermore, with the only usage of `BasePreferences.reload` now gone, the value of that method seems questionable at best. In the event that the functionality is actually needed again, similar to the `ViewHistory`, it'd probably make more sense to simply replace `PDFViewerApplication.preferences` with a new `BasePreferences` instance instead (using e.g. `DefaultExternalServices.createPreferences`).
When updating Preferences using the `set` method, the input is carefully validated. However, no validation is (currently) done when a `BasePreferences` instance is created, which probably isn't that great. Hence this patch that simply ignores, to not unnecessarily break loading of the viewer itself, any invalid Preferences.
|
/botio-linux preview |
From: Bot.io (Linux m4)ReceivedCommand cmd_preview from @timvandermeij received. Current queue size: 0 Live output at: http://54.67.70.0:8877/88368acc38ea502/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.67.70.0:8877/88368acc38ea502/output.txt Total script time: 2.90 mins Published |
timvandermeij
approved these changes
Jul 29, 2018
|
Looks good. Thank you for improving this! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Stop re-loading the Preferences in
PDFViewerApplication.open, and remove theBasePreferences.reloadmethodGiven that the various Preferences are currently, and have been for quite some time, only used when initializing
PDFViewerApplicationre-loading them when a new PDF file is opened in the viewer is essentially a no-op.Furthermore, with the only usage of
BasePreferences.reloadnow gone, the value of that method seems questionable at best. In the event that the functionality is actually needed again, similar to theViewHistory, it'd probably make more sense to simply replacePDFViewerApplication.preferenceswith a newBasePreferencesinstance instead (using e.g.DefaultExternalServices.createPreferences).Validate the Preferences when fetching them from storage
When updating Preferences using the
setmethod, the input is carefully validated. However, no validation is (currently) done when aBasePreferencesinstance is created, which probably isn't that great. Hence this patch that simply ignores, to not unnecessarily break loading of the viewer itself, any invalid Preferences.