Skip to content

Conversation

@ArtificialOwl
Copy link
Member

@ArtificialOwl ArtificialOwl commented Jun 14, 2022

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": "--"
  }
}

Copy link
Member

@PVince81 PVince81 left a comment

Choose a reason for hiding this comment

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

👍 if it works as a quick fix until we find a better solution

@PVince81
Copy link
Member

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
Copy link
Member

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

Copy link
Member

@julien-nc julien-nc left a comment

Choose a reason for hiding this comment

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

Looks good, works fine.

@PVince81
Copy link
Member

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
Copy link
Member

@ArtificialOwl please investigate the failure or find help

@ArtificialOwl ArtificialOwl force-pushed the fix/74/ignore-session branch from 936a3b2 to 69d8fc5 Compare June 22, 2022 11:59
@ArtificialOwl
Copy link
Member Author

ArtificialOwl commented Jun 22, 2022

@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

@ArtificialOwl ArtificialOwl merged commit 23ea022 into master Jun 22, 2022
@delete-merged-branch delete-merged-branch bot deleted the fix/74/ignore-session branch June 22, 2022 12:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants