Skip to content

[tests-only] Ensure that path is a string in makeDavRequest#40128

Merged
individual-it merged 1 commit into
masterfrom
ensure-path-is-string-in-makeDavRequest
Jun 9, 2022
Merged

[tests-only] Ensure that path is a string in makeDavRequest#40128
individual-it merged 1 commit into
masterfrom
ensure-path-is-string-in-makeDavRequest

Conversation

@phil-davis

@phil-davis phil-davis commented Jun 9, 2022

Copy link
Copy Markdown
Contributor

Description

When running the test-runner with PHP 8.1 (against an oC10 system running PHP 7.4) we get:
https://drone.owncloud.com/owncloud/core/36003/82/13

runsh: Total unexpected failed scenarios throughout the test run:
apiWebdavOperations/search.feature:273
apiWebdavOperations/search.feature:289
apiWebdavOperations/search.feature:314
    When user "Alice" searches for resources tagged with tag "JustARegularTag1" using the webDAV API # TagsContext::userSearchesForTagUsingWebDavAPI()
      Type error: str_replace(): Argument #3 ($subject) must be of type array|string, null given (Behat\Testwork\Call\Exception\FatalThrowableError)

Some code passes path=null to WebDavHelper::makeDavRequest, which is allowed. If path is null, then set it to the empty string, so that later uses of path, like str_replace will not complain about a type error.

How Has This Been Tested?

CI - PR #40127 demonstrates that this code fixed the problem.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Database schema changes (next release will require increase of minor version instead of patch)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests only (no source changes)

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised:
  • Changelog item, see TEMPLATE

@phil-davis phil-davis self-assigned this Jun 9, 2022
@phil-davis phil-davis marked this pull request as ready for review June 9, 2022 05:46
@sonarqubecloud

sonarqubecloud Bot commented Jun 9, 2022

Copy link
Copy Markdown

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@individual-it individual-it left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what if its bool or int or something else?
shouldn't $path = (string)$path; do it for all cases

@phil-davis

Copy link
Copy Markdown
Contributor Author

what if its bool or int or something else? shouldn't $path = (string)$path; do it for all cases

The parameter is declared ?string in the function definition. So PHP will already "explode" if anything other than null or a string is passed in.

@phil-davis phil-davis requested a review from individual-it June 9, 2022 06:42
@individual-it individual-it merged commit e3b5879 into master Jun 9, 2022
@delete-merged-branch delete-merged-branch Bot deleted the ensure-path-is-string-in-makeDavRequest branch June 9, 2022 07:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants