After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 785123 - [PATCH] Fix DAV implementation stripping spaces
[PATCH] Fix DAV implementation stripping spaces
Status: RESOLVED FIXED
Product: gvfs
Classification: Core
Component: webdav backend
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gvfs-maint
gvfs-maint
Depends on:
Blocks:
 
 
Reported: 2017-07-19 13:12 UTC by Colin Leroy
Modified: 2017-07-24 13:22 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
The patch (1.53 KB, patch)
2017-07-19 13:12 UTC, Colin Leroy
none Details | Review
patch v2 (1.13 KB, patch)
2017-07-21 11:33 UTC, Colin Leroy
committed Details | Review

Description Colin Leroy 2017-07-19 13:12:20 UTC
Created attachment 355946 [details] [review]
The patch

Hello,

After one of my users created a folder with an extra space at the end of the name on our Nextcloud instance web interface (like "Folder with space "), he complained about not being able to access it using Thunar nor Nautilus.

My debug session revealed that ms_response_get_basename() uses http_path_get_basename() which strips slashes and spaces at the beginning and end of filenames.

Given that the comments in http_path_get_basename() refer to stripping slashes only, I fixed it to remove slashes only, which fixed webdav access to these folders.

I modified the other caller of http_path_get_basename(), http_uri_get_basename(), so that it strips spaces itself, so as not to change its behavior.

The patch is based on current git master.

I hope this helps !

Colin
Comment 1 Ondrej Holy 2017-07-20 14:51:22 UTC
Review of attachment 355946 [details] [review]:

Thanks for your contribution! Looks ok, I don't see any reason, why we should strip the whitespace... will double check with the spec for sure.
Comment 2 Colin Leroy 2017-07-20 14:57:31 UTC
Thanks for your feedback Ondrej :)

I'll wait for it to be officially ack'd and ask the Debian maintainer if it's possible to include it afterwards.
Comment 3 Ondrej Holy 2017-07-21 08:54:02 UTC
Review of attachment 355946 [details] [review]:

I don't see anything relevant in spec, so it should be ok.

I see the following issues with your patch, can you please fix that as per my comments before pushing? Let me know and I will do it myself if you don't want to bother with that.

The commit message should be updated as per the following guidelines (missing "dav:" prefix and bug uri):
https://wiki.gnome.org/Git/CommitMessages

::: daemon/gvfsbackendhttp.c
@@ +137,3 @@
   char *basename;
+  char *stripped_uri_str;
+  

Unwanted spaces on the empty line...

@@ +143,1 @@
+  stripped_uri_str = g_strstrip (g_strdup (uri_str));

This is also probably not needed, because the uri_str always come from libsoup, which strips the whitespace from responses...
Comment 4 Colin Leroy 2017-07-21 11:25:38 UTC
Oh, that simplifies the patch then :)
I'll send you an updated one asap.
Comment 5 Colin Leroy 2017-07-21 11:33:23 UTC
Created attachment 356110 [details] [review]
patch v2

Here is the updated patch addressing your remarks.

Thanks!
Comment 6 Colin Leroy 2017-07-21 11:43:19 UTC
(In reply to Ondrej Holy from comment #3)
 
> I see the following issues with your patch, can you please fix that as per
> my comments before pushing?

I don't know if you meant "before I push" or "before you push", so just in case : 
I don't have git write access on gnome.org :)
Comment 7 Ondrej Holy 2017-07-24 13:22:32 UTC
Thanks a lot!