-
Notifications
You must be signed in to change notification settings - Fork 438
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
[Feature] Provide limit checks when reading global_prefs_override #4916
Comments
@davidpanderson, looks like a valid ticket and it would be nice to have it in the next release. Are you ok to wait for it implementation after #4871 is merged? |
Not really needed. No one AFAIK manually edits global_prefs.xml; |
Some projects customize / modify the standard web code. Coding errors could inadvertently allow users to set illegal values in their computer preferences when editing them on the project web site. Coding errors could also inadvertently cause bad values to be sent in the sched_reply message to the client even if the underlying values were OK. So it would be good to perform sanity checks on the computing prefs values in the sched_reply message as well as when parsing the global_prefs.xml and global_prefs_override.xml files. I believe the same validation code in the client could be used for all 3. |
Describe the problem
Preferences can be modified in three ways:
When changing preferences via the project website or the Manager, those values are validated to be in the correct range. However, when BOINC is started, it will read the global_prefs_override file (if it exists), and store those values. Some of those preferences are currently validated, but not all. The example below shows the percentage of CPUs being changed to a range between 0 and 100:
boinc/lib/prefs.cpp
Lines 509 to 513 in 3fc6c02
However, if suspend_cpu_usage were accidentally edited as a negative number, that negative number would be stored, and BOINC would never crunch any tasks.
Describe the solution you'd like
I feel this can be resolved with two steps:
Additional context
I have started to work on this solution, but with #4871 being in development, I stopped so I didn't have to redo large sections of code. I would be happy to continue developing it after that PR is merged. In the meantime, I'm looking for any feedback.
The text was updated successfully, but these errors were encountered: