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.

Sign up for GitHub

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

A couple of baseurl-related fixes. #19

Merged
merged 4 commits into from
Dec 30, 2022
Merged

A couple of baseurl-related fixes. #19

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
71a7e83
Fixed bytes/str type mismatch when using explicit baseurl.
jaysonlarose Dec 1, 2019
fb33dc7
locks.py:
jaysonlarose Dec 2, 2019
86f8c86
* davcmd.py:
jaysonlarose Dec 9, 2019
1e8c01f
* Corrected is_prefix backwardsness in pywebdav/utils.py (thanks, mik…
jaysonlarose Jul 10, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 9 additions & 9 deletions pywebdav/lib/AuthServer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@
import six.moves.BaseHTTPServer


DEFAULT_AUTH_ERROR_MESSAGE = b"""
DEFAULT_AUTH_ERROR_MESSAGE = """
<head>
<title>%(code)d - %(message)b</title>
<title>%(code)d - %(message)s</title>
</head>
<body>
<h1>Authorization Required</h1>
Expand All @@ -26,7 +26,7 @@


def _quote_html(html):
return html.replace(b"&", b"&amp;").replace(b"<", b"&lt;").replace(b">", b"&gt;")
return html.replace("&", "&amp;").replace("<", "&lt;").replace(">", "&gt;")


class AuthRequestHandler(six.moves.BaseHTTPServer.BaseHTTPRequestHandler):
Expand All @@ -48,7 +48,7 @@ def parse_request(self):
if self.DO_AUTH:
authorization = self.headers.get('Authorization', '')
if not authorization:
self.send_autherror(401, b"Authorization Required")
self.send_autherror(401, "Authorization Required")
return False
scheme, credentials = authorization.split()
if scheme != 'Basic':
Expand All @@ -57,7 +57,7 @@ def parse_request(self):
credentials = base64.decodebytes(credentials.encode()).decode()
user, password = credentials.split(':')
if not self.get_userinfo(user, password, self.command):
self.send_autherror(401, b"Authorization Required")
self.send_autherror(401, "Authorization Required")
return False
return True

Expand All @@ -76,22 +76,22 @@ def send_autherror(self, code, message=None):
try:
short, long = self.responses[code]
except KeyError:
short, long = b'???', b'???'
short, long = '???', '???'
if message is None:
message = short
explain = long
self.log_error("code %d, message %s", code, message)

# using _quote_html to prevent Cross Site Scripting attacks (see bug
# #1100201)
content = (self.error_auth_message_format % {b'code': code, b'message':
_quote_html(message), b'explain': explain})
content = (self.error_auth_message_format % {'code': code, 'message':
_quote_html(message), 'explain': explain})
self.send_response(code, message)
self.send_header('Content-Type', self.error_content_type)
self.send_header('WWW-Authenticate', 'Basic realm="PyWebDAV"')
self.send_header('Connection', 'close')
self.end_headers()
self.wfile.write(content)
self.wfile.write(content.encode('utf-8'))

error_auth_message_format = DEFAULT_AUTH_ERROR_MESSAGE

Expand Down
34 changes: 14 additions & 20 deletions pywebdav/lib/WebDAVServer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@ def send_body(self, DATA, code=None, msg=None, desc=None,
self._send_dav_version()

for a, v in headers.items():
v = v.encode() if isinstance(v, six.text_type) else v
self.send_header(a, v)

if DATA:
Expand Down Expand Up @@ -97,7 +96,9 @@ def send_body(self, DATA, code=None, msg=None, desc=None,

self.end_headers()
if DATA:
if isinstance(DATA, str) or isinstance(DATA, six.text_type) or isinstance(DATA, bytes):
if isinstance(DATA, str):
DATA = DATA.encode('utf-8')
if isinstance(DATA, six.text_type) or isinstance(DATA, bytes):
log.debug("Don't use iterator")
self.wfile.write(DATA)
else:
Expand Down Expand Up @@ -224,8 +225,7 @@ def _HEAD_GET(self, with_body=False):
""" Returns headers and body for given resource """

dc = self.IFACE_CLASS
uri = urllib.parse.urljoin(self.get_baseuri(dc), self.path)
uri = urllib.parse.unquote(uri).encode()
uri = urllib.parse.unquote(urllib.parse.urljoin(self.get_baseuri(dc), self.path))

headers = {}

Expand All @@ -244,7 +244,7 @@ def _HEAD_GET(self, with_body=False):

# get the content type
try:
if uri.endswith(b'/'):
if uri.endswith('/'):
# we could do away with this very non-local workaround for
# _get_listing if the data could have a type attached
content_type = 'text/html;charset=utf-8'
Expand Down Expand Up @@ -331,8 +331,7 @@ def do_PROPFIND(self):
l = self.headers['Content-Length']
body = self.rfile.read(int(l))

uri = urllib.parse.urljoin(self.get_baseuri(dc), self.path)
uri = urllib.parse.unquote(uri).encode()
uri = urllib.parse.unquote(urllib.parse.urljoin(self.get_baseuri(dc), self.path))

try:
pf = PROPFIND(uri, dc, self.headers.get('Depth', 'infinity'), body)
Expand All @@ -341,7 +340,7 @@ def do_PROPFIND(self):
return self.send_status(400)

try:
DATA = b'%s\n' % pf.createResponse()
DATA = pf.createResponse()
Copy link
Owner

Choose a reason for hiding this comment

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

Are you sure the newline isn't still needed here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch.

I'd say it's desirable, and I didn't intend to exclude it... but it didn't break the clients I was using to test (cadaver, davfs2, and GLib gio/gvfs).

Having said that, I'd like to move where that newline's getting generated a bit further down into the implementation, as it feels like it's part of a complete well-formed response. I'm thinking at the end of the return doc.toxml(encoding="utf-8") clauses on lines 119 and 183 of propfind.py. Does that sound reasonable?

(Also, I'm pretty sure the DATA = '%s\n' % rp.createResponse() line just below here in the do_REPORT() method has the same fundamental issue, I just didn't notice it before. I'll make a fix for that, as soon as I figure out how to get a WebDAV client to elicit that response from the server.)

Copy link
Owner

Choose a reason for hiding this comment

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

I get what you mean, this doesn't really look like the right place for it. I think http generally ignores extra newlines anyway so doesn't entirely surprise me the clients still work. Consolidating the place that adds them is a good idea though.

except DAV_Error as error:
(ec, dd) = error.args
return self.send_status(ec)
Expand Down Expand Up @@ -376,8 +375,7 @@ def do_REPORT(self):
l = self.headers['Content-Length']
body = self.rfile.read(int(l))

uri = urllib.parse.urljoin(self.get_baseuri(dc), self.path)
uri = urllib.parse.unquote(uri).encode()
uri = urllib.parse.unquote(urllib.parse.urljoin(self.get_baseuri(dc), self.path))

rp = REPORT(uri, dc, self.headers.get('Depth', '0'), body)

Expand All @@ -403,8 +401,7 @@ def do_MKCOL(self):
return self.send_status(415)

dc = self.IFACE_CLASS
uri = urllib.parse.urljoin(self.get_baseuri(dc), self.path)
uri = urllib.parse.unquote(uri).encode()
uri = urllib.parse.unquote(urllib.parse.urljoin(self.get_baseuri(dc), self.path))

try:
dc.mkcol(uri)
Expand All @@ -419,11 +416,10 @@ def do_DELETE(self):
""" delete an resource """

dc = self.IFACE_CLASS
uri = urllib.parse.urljoin(self.get_baseuri(dc), self.path)
uri = urllib.parse.unquote(uri).encode()
uri = urllib.parse.unquote(urllib.parse.urljoin(self.get_baseuri(dc), self.path))

# hastags not allowed
if uri.find(b'#') >= 0:
if uri.find('#') >= 0:
return self.send_status(404)

# locked resources are not allowed to delete
Expand Down Expand Up @@ -490,8 +486,7 @@ def do_DELETE(self):

def do_PUT(self):
dc = self.IFACE_CLASS
uri = urllib.parse.urljoin(self.get_baseuri(dc), self.path)
uri = urllib.parse.unquote(uri).encode()
uri = urllib.parse.unquote(urllib.parse.urljoin(self.get_baseuri(dc), self.path))

log.debug("do_PUT: uri = %s" % uri)
log.debug('do_PUT: headers = %s' % self.headers)
Expand Down Expand Up @@ -677,12 +672,11 @@ def copymove(self, CLASS):
dc = self.IFACE_CLASS

# get the source URI
source_uri = urllib.parse.urljoin(self.get_baseuri(dc), self.path)
source_uri = urllib.parse.unquote(source_uri).encode()
source_uri = urllib.parse.unquote(urllib.parse.urljoin(self.get_baseuri(dc), self.path))

# get the destination URI
dest_uri = self.headers['Destination']
dest_uri = urllib.parse.unquote(dest_uri).encode()
dest_uri = urllib.parse.unquote(dest_uri)

# check locks on source and dest
if self._l_isLocked(source_uri) or self._l_isLocked(dest_uri):
Expand Down
48 changes: 35 additions & 13 deletions pywebdav/lib/davcmd.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
from .utils import create_treelist, is_prefix
from .errors import *
from six.moves import range
import os

def deltree(dc,uri,exclude={}):
""" delete a tree of resources
Expand Down Expand Up @@ -128,7 +129,7 @@ def copytree(dc,src,dst,overwrite=None):
dc -- dataclass to use
src -- src uri from where to copy
dst -- dst uri
overwrite -- if 1 then delete dst uri before
overwrite -- if True then delete dst uri before

returns dict of uri:error_code tuples from which
another method can create a multistatus xml element.
Expand All @@ -149,8 +150,14 @@ def copytree(dc,src,dst,overwrite=None):
tlist = create_treelist(dc,src)
result = {}

# prepare destination URIs (get the prefix)
dpath = urllib.parse.urlparse(dst)[2]
# Extract the path out of the source URI.
src_path = urllib.parse.urlparse(src).path

# Parse the destination URI.
# We'll be using it to construct destination URIs,
# so we don't just retain the path, like we did with
# the source.
dst_parsed = urllib.parse.urlparse(dst)

for element in tlist:
problem_uris = list(result.keys())
Expand All @@ -160,24 +167,34 @@ def copytree(dc,src,dst,overwrite=None):
# able to copy in problem_uris which is the prefix
# of the actual element. If it is, then we cannot
# copy this as well but do not generate another error.
ok=1
ok=True
for p in problem_uris:
if is_prefix(p,element):
ok=None
ok=False
break

if not ok: continue
if not ok:
continue

# Find the element's path relative to the source.
element_path = urllib.parse.urlparse(element).path
element_path_rel = os.path.relpath(element_path, start=src_path)
# Append this relative path to the destination.
if element_path_rel == '.':
# os.path.relpath("/somedir", start="/somedir") returns
# a result of ".", which we don't want to append to the
# destination path.
dst_path = dst_parsed.path
else:
dst_path = os.path.join(dst_parsed.path, element_path_rel)

# Generate destination URI using our derived destination path.
dst_uri = urllib.parse.urlunparse(dst_parsed._replace(path=os.path.join(dst_parsed.path, element_path_rel)))

# now create the destination URI which corresponds to
# the actual source URI. -> actual_dst
# ("subtract" the base src from the URI and prepend the
# dst prefix to it.)
esrc=element.replace(src,b"")
actual_dst=dpath+esrc

# now copy stuff
try:
copy(dc,element,actual_dst)
copy(dc,element,dst_uri)
except DAV_Error as error:
(ec,dd) = error.args
result[element]=ec
Expand Down Expand Up @@ -216,6 +233,11 @@ def movetree(dc,src,dst,overwrite=None):
# first copy it
res = copytree(dc,src,dst,overwrite)

# TODO: shouldn't we check res for errors and bail out before
# the delete if we find any?
# TODO: or, better yet, is there anything preventing us from
# reimplementing this using `shutil.move()`?

# then delete it
res = deltree(dc,src,exclude=res)

Expand Down
2 changes: 1 addition & 1 deletion pywebdav/lib/davcopy.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,4 +99,4 @@ def tree_action(self):
re.appendChild(st)
ms.appendChild(re)

return doc.toxml(encoding="utf-8")
return doc.toxml(encoding="utf-8") + b"\n"
5 changes: 3 additions & 2 deletions pywebdav/lib/davmove.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
from .constants import COLLECTION, OBJECT, DAV_PROPS
from .constants import RT_ALLPROP, RT_PROPNAME, RT_PROP
from .errors import *
from .utils import create_treelist, quote_uri, gen_estring, make_xmlresponse
from .utils import create_treelist, quote_uri, gen_estring, make_xmlresponse, is_prefix
from .davcmd import moveone, movetree

class MOVE:
Expand Down Expand Up @@ -65,7 +65,8 @@ def tree_action(self):
# (we assume that both uris are on the same server!)
ps=urllib.parse.urlparse(self.__src)[2]
pd=urllib.parse.urlparse(self.__dst)[2]
if ps==pd: raise DAV_Error(403)
if is_prefix(ps, pd):
raise DAV_Error(403)

result=dc.movetree(self.__src,self.__dst,self.__overwrite)
if not result: return None
Expand Down
10 changes: 4 additions & 6 deletions pywebdav/lib/locks.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
from __future__ import absolute_import
import time
from six.moves import urllib
import random
import uuid

import logging

Expand Down Expand Up @@ -95,7 +95,7 @@ def do_UNLOCK(self):
if self._l_isLocked(uri):
self._l_delLock(token)

self.send_body(None, 204, 'Ok', 'Ok')
self.send_body(None, 204, 'OK', 'OK')

def do_LOCK(self):
""" Locking is implemented via in-memory caches. No data is written to disk. """
Expand Down Expand Up @@ -194,9 +194,7 @@ def isValid(self):
return (modified + timeout) > now

def generateToken(self):
_randGen = random.Random(time.time())
return '%s-%s-00105A989226:%.03f' % \
(_randGen.random(),_randGen.random(),time.time())
return str(uuid.uuid4())
Copy link
Owner

Choose a reason for hiding this comment

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

nice cleanup

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. I checked the output from Apache's DAV module and saw that it was returning a UUID, for the lock token, and the example on The top Google hit for "webdav lock token" did the same... since the uuid module is already a standard Python module, I figured it was a win-win.


def getTimeoutString(self):
t = str(self.timeout)
Expand All @@ -211,7 +209,7 @@ def asXML(self, namespace='d', discover=False):
owner_str = ''
if isinstance(self.owner, str):
owner_str = self.owner
elif isinstance(self.owner, xml.dom.minicompat.NodeList):
elif isinstance(self.owner, xml.dom.minicompat.NodeList) and len(self.owner) > 0:
Copy link

Choose a reason for hiding this comment

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

This should be equivalent to just and self.owner, since NodeList inherits from list

owner_str = "".join([node.toxml() for node in self.owner[0].childNodes])

token = self.token
Expand Down
8 changes: 4 additions & 4 deletions pywebdav/lib/propfind.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ def __init__(self, uri, dataclass, depth, body):
self.default_ns = None
self._dataclass = dataclass
self._depth = str(depth)
self._uri = uri.rstrip(b'/')
self._uri = uri.rstrip('/')
self._has_body = None # did we parse a body?

if dataclass.verbose:
Expand Down Expand Up @@ -116,7 +116,7 @@ def create_propname(self):
if uri_childs:
uri_list.extend(uri_childs)

return doc.toxml(encoding="utf-8")
return doc.toxml(encoding="utf-8") + b"\n"

def create_allprop(self):
""" return a list of all properties """
Expand Down Expand Up @@ -180,7 +180,7 @@ def create_prop(self):
if uri_childs:
uri_list.extend(uri_childs)

return doc.toxml(encoding="utf-8")
return doc.toxml(encoding="utf-8") + b"\n"

def mk_propname_response(self, uri, propnames, doc):
""" make a new <prop> result element for a PROPNAME request
Expand Down Expand Up @@ -245,7 +245,7 @@ def mk_prop_response(self, uri, good_props, bad_props, doc):
uri = self._dataclass.baseurl + '/' + '/'.join(uri.split('/')[3:])

# write href information
uparts = urllib.parse.urlparse(uri.decode())
uparts = urllib.parse.urlparse(uri)
fileloc = uparts[2]
href = doc.createElement("D:href")

Expand Down
4 changes: 2 additions & 2 deletions pywebdav/lib/report.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ def create_propname(self):
if uri_childs:
uri_list.extend(uri_childs)

return doc.toxml(encoding="utf-8")
return doc.toxml(encoding="utf-8") + b"\n"

def create_prop(self):
""" handle a <prop> request
Expand Down Expand Up @@ -117,5 +117,5 @@ def create_prop(self):
if uri_childs:
uri_list.extend(uri_childs)

return doc.toxml(encoding="utf-8")
return doc.toxml(encoding="utf-8") + b"\n"

Loading