Skip to content

Conversation

@flymarq
Copy link

@flymarq flymarq commented Jul 25, 2025

The Uri is derived from the request with the method getPath, which generates a valid path fragment suitable for file and directory names.

In the XML-response to a lock-request the uri is used for lockroot element. If the uri contains a space or german umlauts, a strict client like libneon, which is used in davfs2, refuses the lock reponse and files cannot be copied.

flymarq added 2 commits July 25, 2025 12:26
The Uri is derived from the request with the method getPath, which generates a valid path fragment suitable for file and directory names.

In the XML-response to a lock-request the uri is used for lockroot element. If the uri contains a space or german umlauts, a strict client like libneon, which is used in davfs2, refuses the lock reponse and files cannot be copied.
- allow upload of multipe files
- uploads are not limited by php upload size using PUT
- progress bar for each file
@alecpl
Copy link
Contributor

alecpl commented Nov 21, 2025

I see the second commit here is unrelated and should be removed. Also, it would be great to have a test added in tests/Sabre/DAV/Locks/PluginTest.php.

@flymarq
Copy link
Author

flymarq commented Nov 21, 2025

My fault, the second commit does not belong to this issue.

I can try to add a test, but I'm afraid it won't work unless a strict client like libneon performs the test.

@flymarq
Copy link
Author

flymarq commented Nov 21, 2025

Done:

  • remove second commit
  • add LOCK test on url with space with expected http status value 201

Before the patch we received a http status value 423.

]);
$this->server->httpRequest = $request;
$this->server->exec();
self::assertEquals(201, $this->response->status);
Copy link
Contributor

Choose a reason for hiding this comment

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

This test should assert the activelock/lockroot/href property in the XML response. That's where the encoding problem would be visible.

Copy link
Author

Choose a reason for hiding this comment

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

I added a check for the lock token, in the error case no lock token is visiblr.

Copy link
Contributor

Choose a reason for hiding this comment

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

It does not make sense. Your change adds uri property encoding, this is not Lock-Token. Maybe I don't understand something here.

Copy link
Author

Choose a reason for hiding this comment

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

My starting point is this line in the logs of the webserver:
"LOCK /F%20G/F%20G%20H%20J.txt HTTP/1.1" 423 327 "-" "davfs2/1.7.2 neon/0.35.0"

and on the terminal I get
touch: '/home/flynn/media/flynnux/F G/F G H J.txt' kann nicht berührt werden: Eingabe-/Ausgabefehler

In the error case the XML-response contains an exception:
<s:exception>Sabre\DAV\Exception\Conflict no-conflicting-lock>

On success the the token:
<d:locktoken><d:href>opaquelocktoken:c546f02c-5fe7-4a9f-bf78-3bd7c57ef72a</d:href></d:locktoken>

This is, what I try test, tell me if I'm wrong ...

Copy link
Contributor

Choose a reason for hiding this comment

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

ConflictingLock exceptions are thrown in line 170 and 178. Your change is after that, so I don't see how that fixes anything, especially after lockNode().

Copy link
Contributor

@alecpl alecpl Dec 2, 2025

Choose a reason for hiding this comment

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

Also, in the initial pull request description you mention problem with lockroot, so why now you test something different? Seems there might be two different issues.

Copy link
Author

@flymarq flymarq Dec 2, 2025

Choose a reason for hiding this comment

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

The problem is, that the used client (libneon) has strict checking of urls.

The uri in the LOCK request and the url in the xml response differ:

  • the request is url encoded
  • the response before this patch is plain/not url encoded

I already mentioned, that a test is maybe difficult as long as no strict client is used.

Maybe the error above is only consequential error of this problem.

Copy link
Author

Choose a reason for hiding this comment

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

I did not find a method to use the original uri in the lock xml-response, that is maybe the real problem.

}

$this->lockNode($uri, $lockInfo);
$lockInfo->uri = urlencode($lockInfo->uri);
Copy link
Contributor

@alecpl alecpl Dec 1, 2025

Choose a reason for hiding this comment

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

If you do this here you might have it fixed for LOCK request, but what about PROPFIND?
I think the fix should rather go to Sabre\DAV\Xml\Property\LockDiscovery::xmlSerialize(), but I don't really know this code yet. There \Sabre\HTTP\encodePath() should be used, I guess.

Copy link
Author

Choose a reason for hiding this comment

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

I do not have any problmes or errors in PROPFIND calls, why should I change anything?

I can create directories with spaces in the name and inside this directory I can create a file with spaces in the name.

From my experience only LOCK is broken/affected.

@alecpl
Copy link
Contributor

alecpl commented Dec 7, 2025

I was talking about this alecpl@7a549bc

I must say that I have no clue if that does not break other clients. Seems all tests still work.

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.

2 participants