-
Notifications
You must be signed in to change notification settings - Fork 407
Description
I see that the range of task priority is restricted between [0, 100] for the JSON-API user in the input_parser.cpp, but such checks are not provided in the internal C++ code. So, in C++, the range of priority can go high up to the upper limit of uint64.
vroom/src/utils/input_parser.cpp
Lines 112 to 124 in b06900e
| inline Duration get_priority(const rapidjson::Value& object) { | |
| Priority priority = 0; | |
| if (object.HasMember("priority")) { | |
| if (!object["priority"].IsUint()) { | |
| throw Exception(ERROR::INPUT, "Invalid priority value."); | |
| } | |
| priority = object["priority"].GetUint(); | |
| if (priority > MAX_PRIORITY) { | |
| throw Exception(ERROR::INPUT, "Invalid priority value."); | |
| } | |
| } | |
| return priority; | |
| } |
Shall we consider this as a "feature" instead of a bug, and assume that priority checks won't be added in the future in C++? ;-)
This is actually useful if we want to have some "pre-planned tasks" whose schedule won't change. Obviously, if someone is using libvroom, then custom checks can be added on the client-side for priority before sending data to VROOM.
So, on the client-side, one can keep the priority of such pre-planned tasks "very very high" (such as 10^9, instead of 100 because there is no restriction in libvroom), and let the other normal tasks have priority in the range [0, 100]. I understand that this still won't guarantee that pre-planned tasks are always allocated some schedule, but their probability would be very high. And since no checks for priority are added on the VROOM side, this feature can be exploited to make this happen. :-)