nextcloud / files_lock

GNU Affero General Public License v3.0
24 stars 8 forks source link

ignore exception on empty session #75

Closed ArtificialOwl closed 2 years ago

ArtificialOwl commented 2 years ago

So, in case of no user session, this will throw an Exception instead of Throwable, which is then ignored by getLocks()

I am not a huge fan of ignoring locks, but it seems to happens only when upload a new file. Also, this works because we only allow lock on Files and not on Folder (parent).

It seems to not affect the good functioning of the app in the case of creating a new file on a public folder. This is the logs when someone tries to upload a new file on a public folder over a locked existing file (same name, locked by internal user)

{
  "reqId": "zWbuRDPkpK3K7F92jq4p",
  "level": 3,
  "time": "2022-06-14T22:14:55+00:00",
  "remoteAddr": "127.0.0.1",
  "user": "--",
  "app": "webdav",
  "method": "PUT",
  "url": "/public.php/webdav/test.jpg",
  "message": "renaming part file to final file failed $renameOkay: false, $fileExists: true)",
  "userAgent": "Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Firefox/102.0",
  "version": "24.0.2.0"
}
{
  "reqId": "zWbuRDPkpK3K7F92jq4p",
  "level": 4,
  "time": "2022-06-14T22:14:55+00:00",
  "remoteAddr": "127.0.0.1",
  "user": "--",
  "app": "webdav",
  "method": "PUT",
  "url": "/public.php/webdav/test.jpg",
  "message": "Could not rename part file to final file",
  "userAgent": "Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Firefox/102.0",
  "version": "24.0.2.0",
  "exception": {
    "Exception": "Sabre\\DAV\\Exception",
    "Message": "Could not rename part file to final file",
    "Code": 0,
    "Trace": [
      {
        "file": "/home/maxence/sites/nc24/nextcloud/3rdparty/sabre/dav/lib/DAV/Server.php",
        "line": 1137,
        "function": "put",
        "class": "OCA\\DAV\\Connector\\Sabre\\File",
        "type": "->"
      },
      {
        "file": "/home/maxence/sites/nc24/nextcloud/3rdparty/sabre/dav/lib/DAV/CorePlugin.php",
        "line": 492,
        "function": "updateFile",
        "class": "Sabre\\DAV\\Server",
        "type": "->",
        "args": [
          "*** sensitive parameters replaced ***"
        ]
      },
      {
        "file": "/home/maxence/sites/nc24/nextcloud/3rdparty/sabre/event/lib/WildcardEmitterTrait.php",
        "line": 89,
        "function": "httpPut",
        "class": "Sabre\\DAV\\CorePlugin",
        "type": "->"
      },
      {
        "file": "/home/maxence/sites/nc24/nextcloud/3rdparty/sabre/dav/lib/DAV/Server.php",
        "line": 472,
        "function": "emit",
        "class": "Sabre\\DAV\\Server",
        "type": "->"
      },
      {
        "file": "/home/maxence/sites/nc24/nextcloud/3rdparty/sabre/dav/lib/DAV/Server.php",
        "line": 253,
        "function": "invokeMethod",
        "class": "Sabre\\DAV\\Server",
        "type": "->"
      },
      {
        "file": "/home/maxence/sites/nc24/nextcloud/3rdparty/sabre/dav/lib/DAV/Server.php",
        "line": 321,
        "function": "start",
        "class": "Sabre\\DAV\\Server",
        "type": "->"
      },
      {
        "file": "/home/maxence/sites/nc24/nextcloud/apps/dav/appinfo/v1/publicwebdav.php",
        "line": 113,
        "function": "exec",
        "class": "Sabre\\DAV\\Server",
        "type": "->"
      },
      {
        "file": "/home/maxence/sites/nc24/nextcloud/public.php",
        "line": 80,
        "args": [
          "/home/maxence/sites/nc24/nextcloud/apps/dav/appinfo/v1/publicwebdav.php"
        ],
        "function": "require_once"
      }
    ],
    "File": "/home/maxence/sites/nc24/nextcloud/apps/dav/lib/Connector/Sabre/File.php",
    "Line": 355,
    "CustomMessage": "--"
  }
}
PVince81 commented 2 years ago

strange that litmus fails:

3. put_get............... FAIL (GET of `/remote.php/dav/files/admin/litmus/res' failed: 500 Internal Server Error)

was that there before ?

PVince81 commented 2 years ago

@ArtificialOwl can you investigate the failure or check if related at all with your change ?

PVince81 commented 2 years ago

from what I see on master CI was green recently: https://github.com/nextcloud/files_lock/actions/runs/2532691131

so it's likely that this PR breaks something but it's not obvious

PVince81 commented 2 years ago

@ArtificialOwl please investigate the failure or find help

ArtificialOwl commented 2 years ago

@ArtificialOwl please investigate the failure or find help

Were not able myself to locally reproduce the issue with litmus, looks like /rebase on master was enough