Add Webdav-Location header in private link redirect #30383
Conversation
Useful for clients to resolve private links to Webdav URLs without having to extract the file id from the URL
|
If a private link points to a file that is in trash, we don't have a matching Webdav URL yet until #15646 is implemented. Returning a 404 error would be wrong here. |
|
Backport stable10 #30387 |
| return new RedirectResponse($this->urlGenerator->linkToRoute('files.view.index', $params)); | ||
| $response = new RedirectResponse($this->urlGenerator->linkToRoute('files.view.index', $params)); | ||
| if ($isFilesView) { | ||
| $webdavUrl = $this->urlGenerator->linkTo('', 'remote.php') . '/dav/files/' . $uid . '/'; |
There was a problem hiding this comment.
@PVince81 shouldn't this be endpoint-aware instead of hardcoded? i.e. using the same endpoint as the request that originated this $response? My endpoint-juggling context might be playing mind tricks on me, though.
There was a problem hiding this comment.
endpoint-aware in what way ?
The ViewController is not part of Webdav but it's the web page from "index.php/apps/files" so what decision do you think should be made based on this ?
There was a problem hiding this comment.
it's the web page from "index.php/apps/files"
You're right. I was thinking on a different (non-existent) workflow.
What I really meant was If there won't be a problem to use a hardcoded WebDAV path regardless of what the client that is accessing the private link normally uses/understands.
e.g. since this request came from the iOS team and they talk with the old endpoint, the new might offer different properties, no? Or if, like in the past; we detect some problems with the endpoint on the webUI that forces us to temporarily switch back to old, etc.
| $response = new RedirectResponse($this->urlGenerator->linkToRoute('files.view.index', $params)); | ||
| if ($isFilesView) { | ||
| $webdavUrl = $this->urlGenerator->linkTo('', 'remote.php') . '/dav/files/' . $uid . '/'; | ||
| $webdavUrl .= ltrim($baseFolder->getRelativePath($file->getPath()), '/'); |
There was a problem hiding this comment.
@PVince81 take into account that the file name at this point is not URL-encoded: a file called level7$éâ .txt should point to remote.php/dav/files/user/Documents/level7$%C3%A9%C3%A2%20.txt but header is Webdav-Location: /remote.php/dav/files/user/Documents/level7$éâ .txt.
Clients will query a non-existent file then. We should add a test for it as well.
There was a problem hiding this comment.
right, I'll then add some encoding in a subsequent PR
There was a problem hiding this comment.
here we go, please have a look: #30503
|
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Description
Added Webdav-Location header in response when opening private links.
Added 404 status code for error page when requesting inaccessible/invalid private links.
Related Issue
Similar to #28615
Motivation and Context
Useful for clients to resolve private links to Webdav URLs without having to extract the file id from the URL. Please note that there are at least two permalink formats, one with "/f/$fileId" and another one is the link when copied directly from the browser which has "fileid=$fileId". This removes the need for clients to know about this format and simply query whatever link they got and get it resolved.
How Has This Been Tested?
Manual test with curl (with
-Ito only request headers with the "HEAD" method) and unit tests.Screenshots (if appropriate):
Types of changes
Checklist:
cc @michaelstingl @nasli