[pbs-devel] [PATCH proxmox-backup] fix #3526: correctly filter tasks with 'since' and 'until'

[pbs-devel] [PATCH proxmox-backup] fix #3526: correctly filter tasks with 'since' and 'until'

[Dominik Csapak]

The previous assumption was that the Tasks returned by the Iterator are
sorted by the starttime, but that is not actually the case, and
could never have been, since we append the tasks into the log when
they are finished (not started) and running tasks are always iterated
first.

So to correctly filter, we have to go through all tasks that could
fall into the correct time frame, possibly collecting some along the way
that we have to filter out again in the filter_map.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/api2/node/tasks.rs | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/src/api2/node/tasks.rs b/src/api2/node/tasks.rs
index 34e71af1..3d3f7a0a 100644
--- a/src/api2/node/tasks.rs
+++ b/src/api2/node/tasks.rs
@@ -461,7 +461,15 @@ pub fn list_tasks(
         })
         .take_while(|info| {
             match (info, since) {
-                (Ok(info), Some(since)) => info.upid.starttime > since,
+                (Ok(info), Some(since)) => {
+                    // list is really sorted by endtime, so we have
+                    // to take until endtime is smaller than 'since'
+                    if let Some(ref state) = info.state {
+                        state.endtime() > since
+                    } else {
+                        true
+                    }
+                },
                 (Ok(_), None) => true,
                 (Err(_), _) => false,
             }
@@ -472,6 +480,22 @@ pub fn list_tasks(
             Err(_) => return None,
         };
 
+        // we have to filter here again for 'until', because there could have
+        // been some long running active task that could not have been skipped
+        if let Some(until) = until {
+            if info.upid.starttime > until {
+                return None;
+            }
+        }
+
+        // we have to filter again for 'since', because we have collected
+        // until both start and endtime were smaller than since
+        if let Some(since) = since {
+            if info.upid.starttime < since {
+                return None;
+            }
+        }
+
         if !list_all && check_task_access(&auth_id, &info.upid).is_err() {
             return None;
         }
-- 
2.30.2

Re: [pbs-devel] [PATCH proxmox-backup] fix #3526: correctly filter tasks with 'since' and 'until'

[Dietmar Maurer]

I guess this would be easier to understand if you use a "for" loop
instead of combinators. Please can you try?

> On 07/13/2021 3:47 PM Dominik Csapak <d.csapak@proxmox.com> wrote:
> 
>  
> The previous assumption was that the Tasks returned by the Iterator are
> sorted by the starttime, but that is not actually the case, and
> could never have been, since we append the tasks into the log when
> they are finished (not started) and running tasks are always iterated
> first.
> 
> So to correctly filter, we have to go through all tasks that could
> fall into the correct time frame, possibly collecting some along the way
> that we have to filter out again in the filter_map.
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
>  src/api2/node/tasks.rs | 26 +++++++++++++++++++++++++-
>  1 file changed, 25 insertions(+), 1 deletion(-)
> 
> diff --git a/src/api2/node/tasks.rs b/src/api2/node/tasks.rs
> index 34e71af1..3d3f7a0a 100644
> --- a/src/api2/node/tasks.rs
> +++ b/src/api2/node/tasks.rs
> @@ -461,7 +461,15 @@ pub fn list_tasks(
>          })
>          .take_while(|info| {
>              match (info, since) {
> -                (Ok(info), Some(since)) => info.upid.starttime > since,
> +                (Ok(info), Some(since)) => {
> +                    // list is really sorted by endtime, so we have
> +                    // to take until endtime is smaller than 'since'
> +                    if let Some(ref state) = info.state {
> +                        state.endtime() > since
> +                    } else {
> +                        true
> +                    }
> +                },
>                  (Ok(_), None) => true,
>                  (Err(_), _) => false,
>              }
> @@ -472,6 +480,22 @@ pub fn list_tasks(
>              Err(_) => return None,
>          };
>  
> +        // we have to filter here again for 'until', because there could have
> +        // been some long running active task that could not have been skipped
> +        if let Some(until) = until {
> +            if info.upid.starttime > until {
> +                return None;
> +            }
> +        }
> +
> +        // we have to filter again for 'since', because we have collected
> +        // until both start and endtime were smaller than since
> +        if let Some(since) = since {
> +            if info.upid.starttime < since {
> +                return None;
> +            }
> +        }
> +
>          if !list_all && check_task_access(&auth_id, &info.upid).is_err() {
>              return None;
>          }
> -- 
> 2.30.2
> 
> 
> 
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel