Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

webdav: path-escape the lock root #167

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

klarose
Copy link
Contributor

@klarose klarose commented Feb 15, 2023

We ran in to an issue with various microsoft products accessing files
over WebDAV which contain % characters in the name. The requests are
all properly escaped -- e.g. the % is replaced with %25. Further
the basic list functionality of the propfind responses contain encoded
urls.

The lock response/discovery includes the URL (lockroot) to the locked
file. The existing implementation does not path escape that url. This
seems to cause problems with the microsoft applications. When faced with
the unencoded response, they try to lock again and fail. One explanation
for this behaviour is that they maintain a table of locks indexed by
encoded url. When the response from the server doesn't align with the
URL they locked, the application fails.

Update the lock discovery response code to escape the lockroot so that
it matches everything else.

We ran in to an issue with various microsoft products accessing files
over WebDAV which contain `%` characters in the name. The requests are
all properly escaped -- e.g. the `%` is replaced with `%25`. Further
the basic list functionality of the propfind responses contain encoded
urls.

The lock response/discovery includes the URL (lockroot) to the locked
file. The existing implementation does not path escape that url. This
seems to cause problems with the microsoft applications. When faced with
the unencoded response, they try to lock again and fail. One explanation
for this behaviour is that they maintain a table of locks indexed by
encoded url. When the response from the server doesn't align with the
URL they locked, the application fails.

Update the lock discovery response code to escape the lockroot so that
it matches everything else.
@gopherbot
Copy link
Contributor

This PR (HEAD: c72303b) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/net/+/468635 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from Ian Lance Taylor:

Patch Set 1:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/468635.
After addressing review feedback, remember to publish your drafts!

@klarose klarose changed the title path-escape lockroot webdav: path-escape the lock root Feb 16, 2023
@gopherbot
Copy link
Contributor

Message from Kyle Larose:

Patch Set 2:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/468635.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Kyle Larose:

Patch Set 2:

(2 comments)


Please don’t reply on this GitHub thread. Visit golang.org/cl/468635.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Damien Neil:

Patch Set 2:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/468635.
After addressing review feedback, remember to publish your drafts!

The lockroot is a path in the webdav library, meaning it may have
slashes in it. Use the url.URL struct to handle this case when escaping
the lockroot. Also add some unit tests.
@gopherbot
Copy link
Contributor

This PR (HEAD: 806b1f1) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/net/+/468635 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from Ian Lance Taylor:

Patch Set 3: Run-TryBot+1


Please don’t reply on this GitHub thread. Visit golang.org/cl/468635.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gopher Robot:

Patch Set 3:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/468635.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gopher Robot:

Patch Set 3: TryBot-Result+1

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/468635.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Damien Neil:

Patch Set 3: Code-Review+2

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/468635.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Carlos Amedee:

Patch Set 3: Code-Review+1


Please don’t reply on this GitHub thread. Visit golang.org/cl/468635.
After addressing review feedback, remember to publish your drafts!

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