Skip to content
This repository has been archived by the owner on Nov 25, 2020. It is now read-only.

Delete trailing whitespaces on rename and mkdir #1253

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

Conversation

sespivak
Copy link
Contributor

@sespivak sespivak commented Oct 17, 2016

Whitespace at end of filename cause errors while accessing those files
and dirs via WebDAV (e.g. Nautilus, Caja, etc). This patch tries to
avoid creation of such nodes.

@sespivak sespivak force-pushed the clean_trail_whitespace branch 3 times, most recently from c2aa1de to 0ae4217 Compare October 18, 2016 05:21
Whitespace at end of filename cause errors while accessing those files
and dirs via WebDAV (e.g. Nautilus, Caja, etc). This patch tries to
avoid creation of such nodes.
@sespivak sespivak force-pushed the clean_trail_whitespace branch from 0ae4217 to 7cbf214 Compare October 18, 2016 05:25
Windows can't handle files and dirs with trailing whitespace in name
@cdujeu
Copy link
Member

cdujeu commented Oct 18, 2016

Hi @sespivak thanks for your PR, welcome on board!
Just to let you know, not ignoring them, but we are in "code-freeze" mode right now (releasing new major version tomorrow), so we'll have to wait for the next version to properly test and merge.
Don't forget to sign the CLA if it's not already done (see pydio.com > community > contribute page).
cheers
charles

@cdujeu cdujeu added this to the 7.0.1 milestone Oct 18, 2016
@sespivak
Copy link
Contributor Author

Hi, Charles!
Thank you for your answer. I've sign the CLA already.
It's right to test all the changes carefully. My copy of Pydio already patched, so I can wait any period :).

@7omate
Copy link
Contributor

7omate commented Oct 18, 2016

Hi,
I strongly oppose to this behavior.
Trailing white spaces files must produce an ERROR if they can lead to problems, not be silently renamed.

Silently renaming them makes the Sync impossible.

However trimming the names in JS is ok.

@sespivak
Copy link
Contributor Author

sespivak commented Oct 18, 2016

Hi, @7omate

Silently renaming them makes the Sync impossible.

Maybe. It's must be tested, but problem should be solved.
How do you think, rtrimming whitespaces in filenames packed in zip, can cause any problems?

@7omate
Copy link
Contributor

7omate commented Oct 18, 2016

If you unzip the files locally and on your server you would end up with a different result.

@sespivak
Copy link
Contributor Author

If you unzip the files locally and on your server you would end up with a different result.

Yes, you are right. But now if I download folder as zip and folder contains any files with unicode chars in their names, they will recoded by iconv to selected codepage, making filenames different from server version. If filenames will be additionally rtrimmed, it's make files usable in windows.

@7omate
Copy link
Contributor

7omate commented Oct 25, 2016

Hi again,
Sorry for the delay.

  1. In my opinion if the problem is in WebDAV the fix should be in the webDAV plugin.
  2. The other option I think is to allow the creation of nodes but then have an plugin rename all nodes with problematic characters. The plugin should be activated automatically when the workspace has WebDAV active.

How hard would those solutions be?
@cdujeu @sespivak

@sespivak
Copy link
Contributor Author

Hi, @7omate !

The problem not only in WebDAV, but in handling files and dirs with trailing whitespaces in Windows too.
Those files can be created, deleted, but can't be accessed or renamed. Checked in Windows XP.
So, these files shouldn't be created.

@cdujeu
Copy link
Member

cdujeu commented Nov 20, 2016

@7omate retesting this use case with my Windows 7, I have a sync with jpg file set up. If I rename the file with a trailing space after the jpg extension, it then just... disappear from the local folder on windows!!
Did you do some test on your side? It think that's a real issue here :-)
Tested with client 1.2.8

cdujeu added a commit that referenced this pull request Nov 21, 2016
…reating a content instead of path in some case (trailing spaces). See #1253
ghecquet added a commit that referenced this pull request Nov 21, 2016
…elop

* 'develop' of http://github.com/pydio/pydio-core:
  Initiate release note and dist resources for 7.0.2
  RolesManager loading role data : Make sure to remove invalid repos on error from shared repositories list.
  i18n new prompt exception mechanism
  Update trigger as mysql automatically pads varchars when comparing, creating a content instead of path in some case (trailing spaces). See #1253
  Manual merge of #1292 (put encoding call directly inside win-dedicated function.
  Auth.ldap: add starttsl support. See #1248, should fix it.
  Fix zip operation when child drivers are remote (inc. smb) - Fix #1287 Fix base detection for archive
  Fix SMB access driver and custom smbclient path. Close #1290
  Rewire ALLOWED_EXTENSIONS configs - Fix #1283
  Fix #1288
  Fix Imap & EML plugins - Warning php7 requires Mail_mimeDecode 1.5.6 - Fix #1282
@cdujeu
Copy link
Member

cdujeu commented Nov 22, 2016

Not fully fixed yet, but at least we fixed an issue with the SQL trigger that would create an invalid change (content instead of path) with mysql when renaming a file with a trailing space.
To be continued...

@cdujeu cdujeu modified the milestones: 7.2.X, 7.0.2 Nov 22, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants