search

BOINC / boinc

Open-source software for volunteer computing and grid computing.
https://boinc.berkeley.edu
GNU Lesser General Public License v3.0
2.01k stars 445 forks source link

[Feature] Provide limit checks when reading global_prefs_override #4916

Open Vulpine05 opened 2 years ago

Vulpine05 commented 2 years ago

Describe the problem Preferences can be modified in three ways:

  1. Web preferences
  2. Locally, via the Manger
  3. Locally, by editing the global_prefs_override.xml file directly

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: https://github.com/BOINC/boinc/blob/3fc6c024711175413eb9e57c53fad1a78267274e/lib/prefs.cpp#L509-L513

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:

  1. For any double variables that don't have limits set already, add them. These limits are likely to be simple, such as setting a negative number to 0. This will have to be evaluated on a case-by-case basis.
  2. The user should be aware that BOINC had to modify one of their values. I think the best thing would be to create a message in the notices tab. I would think this would be similar to how a message appears when there is incorrect data in an app_config file. The message doesn't have to be too specific, perhaps which preference was changed. I would assume if the user is editing their global_prefs_override file, they will know how to read through it and find the error.

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.

AenBleidd commented 2 years ago

@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?

davidpanderson commented 2 years ago

Not really needed. No one AFAIK manually edits global_prefs.xml; it's not documented except for an ancient trac page.

Vulpine05 commented 2 years ago

Not really needed. No one AFAIK manually edits global_prefs.xml; it's not documented except for an ancient trac page.

Actually, there is documentation here and here.

CharlieFenton commented 2 years ago

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 _schedreply 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 _schedreply message as well as when parsing the _globalprefs.xml and _global_prefsoverride.xml files. I believe the same validation code in the client could be used for all 3.