fix: :bug: Update pdf save to use correct dav path and credentials by moodyjmz · Pull Request #1414 · nextcloud/files_pdfviewer

moodyjmz · 2026-04-10T11:43:03Z

The dav path is legacy when saving a pdf. This becomes an issue with password protected public shares.

We need to provide the request token for public

for user shares, we supply the current user id - the auth etc happens server side via session

To test this change

Place a pdf into a folder. Make this folder public, but password protected and editable.

Open using the share link
Edit the pdf
save it.
It should now save.

@icewind1991 was instrumental in pointing me in the right direction:)

moodyjmz · 2026-04-14T09:39:19Z

@susnux Is this ok now?

danxuliu

Thanks for the fix! I still need to do a proper review, but a couple of points :-)

When addressing a review and fixing an issue in the original commits please use a fixup commit instead and squash the commits once the review is approved. In some cases you might need to rebase and amend the original commit if there are conflicts with later commits or you need to adjust the commit description, but at least in this specific pull request a fixup commit would be fine. Of course you can always squash the commits even if they were not originally created as fixup commits.
Please do not merge the latest main branch into the pull request, but rebase the commits on the latest main branch instead.

Independently of that, once the reviews are approved and the commits squashed in the PDF viewer app you will still need to build the JavaScript files. You can do that by writing a comment in GitHub with /compile /, which will trigger the compile bot and automatically add the needed files in its own commit.

If you use /compile amend / it will amend the last commit instead, but it is better to use /compile / and add the changes in its own commit to ease backports (which can be done also with GitHub comments like /backport to stable32 or, if you want that our future bot overlords see that you were already polite to them back in the day, /backport to stable32 please).

moodyjmz · 2026-04-14T12:29:53Z

Thanks for the fix! I still need to do a proper review, but a couple of points :-)

When addressing a review and fixing an issue in the original commits please use a fixup commit instead and squash the commits once the review is approved. In some cases you might need to rebase and amend the original commit if there are conflicts with later commits or you need to adjust the commit description, but at least in this specific pull request a fixup commit would be fine. Of course you can always squash the commits even if they were not originally created as fixup commits.

Please do not merge the latest main branch into the pull request, but rebase the commits on the latest main branch instead.

Independently of that, once the reviews are approved and the commits squashed in the PDF viewer app you will still need to build the JavaScript files. You can do that by writing a comment in GitHub with /compile /, which will trigger the compile bot and automatically add the needed files in its own commit.

If you use /compile amend / it will amend the last commit instead, but it is better to use /compile / and add the changes in its own commit to ease backports (which can be done also with GitHub comments like /backport to stable32 or, if you want that our future bot overlords see that you were already polite to them back in the day, /backport to stable32 please).

Ah, cool, thanks for the pointers, I'll be sure to do that

moodyjmz · 2026-05-06T05:34:49Z

@susnux would you be able to review this?

danxuliu

As far as I can see the actual fix is using the dav endpoint rather than the webdav one (which is the legacy endpoint). Do you mind if I split your commit in a fix replacing the endpoint and a refactor unifying getRootPath for future reference? :-)

danxuliu · 2026-05-06T12:21:48Z

 		headers: {
 			'Content-Type': 'application/pdf',
-			// Not needed for public pages, although there is no problem if it
-			// is set.


Is it actually needed for public pages now? At least in a quick test it seems to be working fine even if requesttoken is not passed 🤔

Your domain knowledge is better here than mine, I would have to dive back into the code and refresh myself 😬

Not really... I asked because I do not really know and I did not want to dive into the server code myself 😅

I would need to test this again to see

@susnux do you want the getRootPath change in this PR but with a separate commit? (I presume for CL)

@susnux I made that change. I tested:

auth'd user needs the token
public not

previously it was sent for both, I changed it so it is only sent for auth'd users

Sorry for the delay. I just restored the comment about requesttoken being unneeded and removed it in the last commit which is the one addressing it, and added an explicit commit for what susnux mentioned as it was the case before this pull request.

Other than that, all good, thanks a lot for addressing everything! :-)

moodyjmz · 2026-05-06T12:41:10Z

As far as I can see the actual fix is using the dav endpoint rather than the webdav one (which is the legacy endpoint). Do you mind if I split your commit in a fix replacing the endpoint and a refactor unifying getRootPath for future reference? :-)

sure

moodyjmz · 2026-05-11T12:37:22Z

with the recent changes, is that good now?

camilasan · 2026-05-12T10:27:45Z

with the recent changes, is that good now?

@susnux @danxuliu @szaimen ^^
I need this PR...

Use /public.php/dav/files/SHARING_TOKEN instead of the legacy /public.php/webdav endpoint for public share uploads. Signed-off-by: James Manuel <moodyjmz@users.noreply.github.com>

This should not happen, but if it does, it should throw to avoid silently resolving to "/files/undefined" and hiding the error. Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de> Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>

Inline getUserRoot into getRootPath so both public and private paths are resolved in one place. Signed-off-by: James Manuel <moodyjmz@users.noreply.github.com>

requesttoken is for CSRF protection and is only meaningful when there is an authenticated session. Public share uploads authenticate via the token in the URL path, so the header is not needed there. Signed-off-by: James Manuel <moodyjmz@users.noreply.github.com>

danxuliu · 2026-05-12T15:58:49Z

/compile /

danxuliu · 2026-05-12T15:59:13Z

/backport to stable33 please

danxuliu · 2026-05-12T15:59:20Z

/backport to stable32 please

danxuliu

Tested and works 👍

Signed-off-by: nextcloud-command <nextcloud-command@users.noreply.github.com>

moodyjmz self-assigned this Apr 10, 2026

susnux requested review from danxuliu and skjnldsv April 10, 2026 12:42

susnux added bug 3. to review labels Apr 10, 2026

susnux reviewed Apr 10, 2026

View reviewed changes

Comment thread src/utils/davUtils.js Outdated

moodyjmz requested a review from susnux April 10, 2026 14:23

danxuliu requested changes Apr 14, 2026

View reviewed changes

moodyjmz force-pushed the fix/dav-save-issue branch from 925f169 to 889591d Compare May 6, 2026 05:34

danxuliu reviewed May 6, 2026

View reviewed changes

moodyjmz force-pushed the fix/dav-save-issue branch 4 times, most recently from 70d5d5f to 52555c4 Compare May 7, 2026 12:03

moodyjmz and others added 4 commits May 12, 2026 17:30

fix: Update pdf save to use correct dav path and credentials

d6a950d

Use /public.php/dav/files/SHARING_TOKEN instead of the legacy /public.php/webdav endpoint for public share uploads. Signed-off-by: James Manuel <moodyjmz@users.noreply.github.com>

refactor: Unify getRootPath and remove getUserRoot

0ab8cb1

Inline getUserRoot into getRootPath so both public and private paths are resolved in one place. Signed-off-by: James Manuel <moodyjmz@users.noreply.github.com>

danxuliu force-pushed the fix/dav-save-issue branch from 52555c4 to 0506d9a Compare May 12, 2026 15:54

backportbot Bot added the backport-request label May 12, 2026

danxuliu approved these changes May 12, 2026

View reviewed changes

danxuliu enabled auto-merge May 12, 2026 15:59

chore(assets): Recompile assets

a666764

Signed-off-by: nextcloud-command <nextcloud-command@users.noreply.github.com>

danxuliu merged commit 1373fd8 into master May 12, 2026
36 checks passed

danxuliu deleted the fix/dav-save-issue branch May 12, 2026 16:08

This was referenced May 12, 2026

[stable32] fix: 🐛 Update pdf save to use correct dav path and credentials #1439

Merged

[stable33] fix: 🐛 Update pdf save to use correct dav path and credentials #1440

Merged

backportbot Bot removed the backport-request label May 12, 2026

blizzz mentioned this pull request May 12, 2026

34.0.0 beta 5 nextcloud/server#60334

Merged

Conversation

moodyjmz commented Apr 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

moodyjmz commented Apr 14, 2026

Uh oh!

danxuliu left a comment

Choose a reason for hiding this comment

Uh oh!

moodyjmz commented Apr 14, 2026

Uh oh!

moodyjmz commented May 6, 2026

Uh oh!

danxuliu left a comment

Choose a reason for hiding this comment

Uh oh!

danxuliu May 6, 2026

Choose a reason for hiding this comment

Uh oh!

moodyjmz May 6, 2026

Choose a reason for hiding this comment

Uh oh!

danxuliu May 6, 2026

Choose a reason for hiding this comment

Uh oh!

moodyjmz May 6, 2026

Choose a reason for hiding this comment

Uh oh!

moodyjmz May 7, 2026

Choose a reason for hiding this comment

Uh oh!

moodyjmz May 7, 2026

Choose a reason for hiding this comment

Uh oh!

danxuliu May 12, 2026

Choose a reason for hiding this comment

Uh oh!

moodyjmz commented May 6, 2026

Uh oh!

moodyjmz commented May 11, 2026

Uh oh!

camilasan commented May 12, 2026

Uh oh!

danxuliu commented May 12, 2026

Uh oh!

danxuliu commented May 12, 2026

Uh oh!

danxuliu commented May 12, 2026

Uh oh!

danxuliu left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

moodyjmz commented Apr 10, 2026 •

edited

Loading