A couple of baseurl-related fixes. #19
Conversation
| @@ -341,7 +340,7 @@ def do_PROPFIND(self): | |||
| return self.send_status(400) | |||
|
|
|||
| try: | |||
| DATA = b'%s\n' % pf.createResponse() | |||
| DATA = pf.createResponse() | |||
There was a problem hiding this comment.
Are you sure the newline isn't still needed here?
There was a problem hiding this comment.
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.)
There was a problem hiding this comment.
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.
| _randGen = random.Random(time.time()) | ||
| return '%s-%s-00105A989226:%.03f' % \ | ||
| (_randGen.random(),_randGen.random(),time.time()) | ||
| return str(uuid.uuid4()) |
There was a problem hiding this comment.
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.
| @@ -59,6 +59,9 @@ mimecheck = 1 | |||
| # webdav level (1 = webdav level 2) | |||
| lockemulation = 1 | |||
|
|
|||
| # dav server base url | |||
| baseurl = | |||
There was a problem hiding this comment.
Did you mean to leave this empty config line here?
There was a problem hiding this comment.
Yes. Without that directive in the config file, davserver blows up with a configparser.NoOptionError:
[09:22] jayson@skully$ davserver -c wut_it_be/config.ini
2019-12-03 09:22:59,150 INFO Starting up PyWebDAV server (version 0.9.14)
2019-12-03 09:22:59,151 INFO chunked_http_response feature ON
2019-12-03 09:22:59,151 INFO http_request_use_iterator feature OFF
2019-12-03 09:22:59,151 INFO http_response_use_iterator feature ON
2019-12-03 09:22:59,151 INFO Initialized with /home/jayson/wut_it_be http://localhost:8081/
2019-12-03 09:22:59,151 WARNING Authentication disabled!
2019-12-03 09:22:59,151 INFO Serving data from /home/jayson/wut_it_be
Traceback (most recent call last):
File "/usr/lib/python3.7/configparser.py", line 788, in get
value = d[option]
File "/usr/lib/python3.7/collections/__init__.py", line 914, in __getitem__
return self.__missing__(key) # support subclasses that define __missing__
File "/usr/lib/python3.7/collections/__init__.py", line 906, in __missing__
raise KeyError(key)
KeyError: 'baseurl'
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "/usr/local/bin/davserver", line 11, in <module>
load_entry_point('PyWebDAV3==0.9.14', 'console_scripts', 'davserver')()
File "/usr/local/lib/python3.7/dist-packages/PyWebDAV3-0.9.14-py3.7.egg/pywebdav/server/server.py", line 376, in run
handler=handler)
File "/usr/local/lib/python3.7/dist-packages/PyWebDAV3-0.9.14-py3.7.egg/pywebdav/server/server.py", line 94, in runserver
if handler._config.DAV.baseurl:
File "/usr/local/lib/python3.7/dist-packages/PyWebDAV3-0.9.14-py3.7.egg/pywebdav/lib/INI_Parse.py", line 34, in __getattr__
return self.__parser.get(self.name, name)
File "/usr/lib/python3.7/configparser.py", line 791, in get
raise NoOptionError(option, section)
configparser.NoOptionError: No option 'baseurl' in section: 'DAV'
There was a problem hiding this comment.
I thought that's why this is PyWebDAV3 :)
To be honest, I didn't even think about python 2 compatibility when I submitted this pull request. I figured the references to six might have been part of the old PyWebDAV codebase (which I promptly set a figurative match to as soon as I saw that this library existed and I didn't have to fix the whole thing).
But seriously, I feel you... I'm looking forward to saying goodbye to all the insanity that comes with maintaining Python2 compatibility (and maintaining Python2 text objects).
According to https://pythonclock.org/, only about 28 more days to go! Which means if Python 2 were a cop in a movie, now's about the time it gets tragically gunned down in the line of duty...
There was a problem hiding this comment.
Just a heads-up: it looks like there's a destructive bug in do_MOVE.
I tried to move a populated directory into another directory, and wound up with an empty source directory inside the destination.
Call me crazy, but I think it's related to that blurb in davcmd.movetree() that goes "PROBLEM: if something did not copy then we have a problem when deleting as the original might get deleted!" I'm looking into it.
|
When I first made this port I was stuck with py2 on my work project, so cared about back compatibility. Those days are long gone however so I'm not fussed. Thanks for the fix up work, there's always been lingering str/bytes issues here. I'm not actually using this project myself so haven't found the time to systematically clean it up myself. I actually only ported this server in the first place because it was used in the test suite of a webdav client I was porting to py3, so I did this too just to run the tests! Irrespective of that though I'm happy to take contributions and make releases, they're pretty much automated anyway. |
|
Yeah, I hadn't touched any WebDAV stuff myself in a couple of years, and then I had to get some files off my phone. And, for whatever reason, WebDAV somehow became the lowest common denominator as the network-filesystem-like-thing that iOS apps use. |
|
I've fixed the "move command fails destructively" issue, finished the XML newline cleanup, and made a few other minor tweaks. I think everything's in a state that it's OK to merge in, if you want to take another look. --Jays |
|
I haven't done any great amount of testing, but trying to move a collection to itself should no longer be destructive. |
|
Great, @mik2k2 would you be able to test this? If you're both happy is be happy to merge this into a new release. |
| @@ -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: | |||
There was a problem hiding this comment.
This should be equivalent to just and self.owner, since NodeList inherits from list
| return 1 | ||
| else: | ||
| return None | ||
| """ returns 1 if uri1 is a prefix of uri2 """ |
|
In |
Added baseurl directive to example config.ini.
Changed "Ok" to "OK" in do_UNLOCK send_body response. Changed erroneous string HTTP response codes to integer. Changed the... strange thing... that generateToken outputs to a uuid.uuid4. Added a length check to avoid an error in asXML. fshandler.py: Removed `.encode()`/`.decode()` stuff in relation to URIs; URIs should be of type `str`. propfind.py: Reverted the hacky `.encode` I added in my previous fix, removed the `.decode()` further down; URIs should be of type `str`. WebDAVServer.py: Removed a lot of conversions of URI to bytes; URIs should be of type `str`. Removed a clause in `DAVRequestHandler.send_body()` that was converting headers to `bytes` before sending them. This was causing many headers to be sent across the line looking like `Connection: b'Keep-Alive'`. Added a clause to `DAVRequestHandler.send_body()` to encode DATA to `bytes`; all XML generated is kept as `str` until just before it is to be sent over the line. AuthServer.py: Changed a bunch of `bytes` objects over to regular strings. Added a bit at the end of `AuthRequestHandler.send_autherror()` to encode its HTML response to `bytes` just before it gets sent over the line.
* reimplemented `copytree()` URI derivation logic using `urllib.parse`
and `os.path`.
* WebDAVServer.py, propfind.py, davcopy.py, utils:
* Moved the responsibility of adding newlines to the end of response
XML out of WebDAVServer.py and into the XML generation routines in
propfind.py, davcopy.py, and utils.py.
* utils.py:
* Altered `is_prefix()` to use `os.path.commonpath()` instead of
a substring comparison.
…2k2!) * Fixed a bug where attempting to move a collection to itself would result in homicide.
|
Thanks, I'm going to merge as-is and fix comments separately! |
|
Included in https://pypi.org/project/PyWebDAV3/0.10.0/ |
I addressed this and a couple of your other comments here in follow up commits included in https://pypi.org/project/PyWebDAV3/0.10.0/ |
Fixed bytes/str type mismatch when using explicit baseurl.
Added baseurl directive to example config.ini.