Avoid invalid time-range

[phil-davis]

Issue: sabre-io/Baikal#929

It is possible to call calendarQuery with all sorts of not-so-valid/expected stuff in $filters array.

In particular, it seems that time-range gets a boolean value (probably false ?) in some cases, rather than being an array with some of start and end keys. That causes PHP 7.4 to complain when we try to access things like $timeRange['start']

When this happens, set it to an array. If start or end is missing, set to false so that these keys exist for the later code to process.

Note: there are plenty of other assumptions made about what exists $filters array passed to calendarQuery, but I am not attempting to fix the whole universe. For example:

        // if no filters were specified, we don't need to filter after a query
        if (!$filters['prop-filters'] && !$filters['comp-filters']) {
            $requirePostFilter = false;
        }

If you specify comp-filters but leave out prop-filters completely, then a message will be emitted on PHP 7.4 complaining about the reference to non-existent array key prop-filters.

There are lots of places that can have array_key_exists or isset checks, and either fill in empty default values or throw new \InvalidArgumentException

[codecov]

Codecov Report

Merging #1275 (8eb1f08) into master (d385f9b) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##             master    #1275   +/-   ##
=========================================
  Coverage     97.12%   97.12%           
- Complexity     2775     2778    +3     
=========================================
  Files           174      174           
  Lines          8025     8030    +5     
=========================================
+ Hits           7794     7799    +5     
  Misses          231      231           

Impacted Files	Coverage Δ	Complexity Δ
lib/CalDAV/Backend/PDO.php	99.17% <100.00%> (+<0.01%)	133.00 <0.00> (+3.00)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d385f9b...8eb1f08. Read the comment docs.

[phil-davis]

Note: I can add some unit test cases tomorrow.

[phil-davis]

Oh, "tomorrow" was 4 days ago! I will look at the weekend.

[phil-davis]

Sorry about the cut-paste in the unit tests. But there were already a bunch of test cases like that. It could have some dataProvider put in, but that's a refactoring for another day.

@ByteHamster @DeepDiver1975 @staabm or anyone, please review.

[ByteHamster]

Sorry for the late reply. The change looks a bit hacky, to be honest, but I do not know enough about sabre/dav to be able to properly review this PR.

[staabm]

random idea: What do you think about throwing exceptions on invalid input instead?

Related: I would love to get a more specific phpdoc for $filter, so phpstan/psalm could tell the caller about invalid parameters at analysis time

[phil-davis]

It was trying to do something "reasonable" if a caller passes $filters array that does not the "expected" time-range as an array. Then set the time-range to have start and end false - effectively no time-range.

Actually there are plenty of things that might or might not be passed in $filters - it is just declared as an array and the existing code does not validate it much. PHP 7.4 now complains more about attempts t do array access on objects that are not arrays.

Of course it would also be good to find out what the caller has been sending in $filters all this time - it used to "work" in some way until PHP 7.4 started complaining.

Actually there might be plenty more places like this. I will have another look tomorrow.

[phil-davis]

See #1319