Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Call to a member function getUserId() on bool #4170

Closed
wants to merge 1 commit into from

Conversation

solracsf
Copy link
Member

📝 Summary

Check that $currentSession is not false before trying to retrieve the user.

🏁 Checklist

  • Code is properly formatted (npm run lint / npm run stylelint / composer run cs:check)
  • Sign-off message is added to all commits
  • Tests (unit, integration and/or end-to-end) passing and the changes are covered with tests
  • Documentation (README or documentation) has been updated or is not required

Signed-off-by: Git'Fellow <12234510+solracsf@users.noreply.github.com>
@solracsf solracsf added this to the Nextcloud 27 milestone May 11, 2023
@cypress
Copy link

cypress bot commented May 11, 2023

1 flaky tests on run #9809 ↗︎

0 142 1 0 Flakiness 1

Details:

fix: Call to a member function getUserId() on bool
Project: Text Commit: bf4eb39fb5
Status: Passed Duration: 05:50 💡
Started: May 11, 2023 1:14 PM Ended: May 11, 2023 1:20 PM
Flakiness  cypress/e2e/sync.spec.js • 1 flaky test

View Output Video

Test Artifacts
Sync > recovers from a lost connection Output Screenshots

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.

@solracsf solracsf requested a review from juliusknorr May 11, 2023 13:22
$this->userToRestore = $this->userSession->getUser();
$this->userSession->setUser($user);
$userId = $currentSession->getUserId();
if ($userId !== false) {
Copy link
Member

Choose a reason for hiding this comment

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

I'm curious how we end up with a boolean here, since this should rather be null

Copy link
Member

Choose a reason for hiding this comment

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

protected ?string $userId = null;

Copy link
Member

Choose a reason for hiding this comment

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

Do you have an exception trace or error log of this being an issue?

Copy link
Member Author

Choose a reason for hiding this comment

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

{
  "reqId": "6ir0CrrQgBReHnset6dX",
  "level": 3,
  "time": "2023-05-12T18:31:27+02:00",
  "remoteAddr": "195.20.18.18",
  "user": "elith.breur",
  "app": "index",
  "method": "POST",
  "url": "/apps/text/session/sync",
  "message": "Call to a member function getUserId() on bool in file '/var/www/nextcloud/apps/text/lib/Controller/SessionController.php' line 111",
  "userAgent": "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/113.0.0.0 Safari/537.36",
  "version": "24.0.12.1",
  "exception": {
    "Exception": "Exception",
    "Message": "Call to a member function getUserId() on bool in file '/var/www/nextcloud/apps/text/lib/Controller/SessionController.php' line 111",
    "Code": 0,
    "Trace": [
      {
        "file": "/var/www/nextcloud/lib/private/AppFramework/App.php",
        "line": 172,
        "function": "dispatch",
        "class": "OC\\AppFramework\\Http\\Dispatcher",
        "type": "->",
        "args": [
          [
            "OCA\\Text\\Controller\\SessionController"
          ],
          "sync"
        ]
      },
      {
        "file": "/var/www/nextcloud/lib/private/Route/Router.php",
        "line": 298,
        "function": "main",
        "class": "OC\\AppFramework\\App",
        "type": "::",
        "args": [
          "OCA\\Text\\Controller\\SessionController",
          "sync",
          [
            "OC\\AppFramework\\DependencyInjection\\DIContainer"
          ],
          [
            "text.Session.sync"
          ]
        ]
      },
      {
        "file": "/var/www/nextcloud/lib/base.php",
        "line": 1030,
        "function": "match",
        "class": "OC\\Route\\Router",
        "type": "->",
        "args": [
          "/apps/text/session/sync"
        ]
      },
      {
        "file": "/var/www/nextcloud/index.php",
        "line": 36,
        "function": "handleRequest",
        "class": "OC",
        "type": "::",
        "args": []
      }
    ],
    "File": "/var/www/nextcloud/lib/private/AppFramework/Http/Dispatcher.php",
    "Line": 165,
    "Previous": {
      "Exception": "Error",
      "Message": "Call to a member function getUserId() on bool",
      "Code": 0,
      "Trace": [
        {
          "file": "/var/www/nextcloud/apps/text/lib/Controller/SessionController.php",
          "line": 105,
          "function": "loginSessionUser",
          "class": "OCA\\Text\\Controller\\SessionController",
          "type": "->",
          "args": [
            "*** sensitive parameters replaced ***"
          ]
        },
        {
          "file": "/var/www/nextcloud/lib/private/AppFramework/Http/Dispatcher.php",
          "line": 225,
          "function": "sync",
          "class": "OCA\\Text\\Controller\\SessionController",
          "type": "->",
          "args": [
            "*** sensitive parameters replaced ***",
            "*** sensitive parameters replaced ***",
            "*** sensitive parameters replaced ***",
            0,
            null,
            false,
            false
          ]
        },
        {
          "file": "/var/www/nextcloud/lib/private/AppFramework/Http/Dispatcher.php",
          "line": 133,
          "function": "executeController",
          "class": "OC\\AppFramework\\Http\\Dispatcher",
          "type": "->",
          "args": [
            [
              "OCA\\Text\\Controller\\SessionController"
            ],
            "sync"
          ]
        },
        {
          "file": "/var/www/nextcloud/lib/private/AppFramework/App.php",
          "line": 172,
          "function": "dispatch",
          "class": "OC\\AppFramework\\Http\\Dispatcher",
          "type": "->",
          "args": [
            [
              "OCA\\Text\\Controller\\SessionController"
            ],
            "sync"
          ]
        },
        {
          "file": "/var/www/nextcloud/lib/private/Route/Router.php",
          "line": 298,
          "function": "main",
          "class": "OC\\AppFramework\\App",
          "type": "::",
          "args": [
            "OCA\\Text\\Controller\\SessionController",
            "sync",
            [
              "OC\\AppFramework\\DependencyInjection\\DIContainer"
            ],
            [
              "text.Session.sync"
            ]
          ]
        },
        {
          "file": "/var/www/nextcloud/lib/base.php",
          "line": 1030,
          "function": "match",
          "class": "OC\\Route\\Router",
          "type": "->",
          "args": [
            "/apps/text/session/sync"
          ]
        },
        {
          "file": "/var/www/nextcloud/index.php",
          "line": 36,
          "function": "handleRequest",
          "class": "OC",
          "type": "::",
          "args": []
        }
      ],
      "File": "/var/www/nextcloud/apps/text/lib/Controller/SessionController.php",
      "Line": 111
    },
    "CustomMessage": "--"
  },
  "id": "6461e4a78f770"
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

The trace looks like it's from a Nextcloud 24 install. So we need to reference the stable24 branch to see the actual code.

This exception originates here:
https://github.com/nextcloud/text/blob/stable24/lib/Controller/SessionController.php#L111

Here $currentSession is undefined. We already have a fix for this with the check in line 119.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks like the issue in this trace was fixed in 6640ad2

could you try upgrading to Nextcloud 25 and see if that fixes it?

Copy link
Member Author

Choose a reason for hiding this comment

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

Okay. Upgrade is planned with 25.0.7, will report back.

@juliusknorr juliusknorr added the bug Something isn't working label May 17, 2023
Copy link
Collaborator

@max-nextcloud max-nextcloud left a comment

Choose a reason for hiding this comment

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

It's not clear to me if this actually fixes an issue.

Could you check and clarify if it still does?

@blizzz blizzz modified the milestones: Nextcloud 27, Nextcloud 28 May 23, 2023
@juliusknorr
Copy link
Member

Let's close this for now.

@solracsf Feel free to reopen and adjust if there is a related fix for the main branch still needed.

@juliusknorr juliusknorr closed this Jun 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants