Piwigo Bugtracker

Piwigo bug tracker has moved to Github

This bugtracker is kept to provide history on old issues.


View Issue Details Jump to Notes ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0003030Piwigosynchronizationpublic2014.01.23 12:062015.02.10 09:39
ReporterJanisV 
Assigned To 
PrioritynormalSeveritycrashReproducibilityalways
StatusnewResolutionopen 
PlatformOSOS Version
Product Version2.6.0 
Target Version2.8.0beta1Fixed in Version 
Summary0003030: Sync fail with files containing ' (apostrophe) in filename
DescriptionSync any folder with file "Kamen' at Nameless.JPG" and $conf['sync_chars_regex'] = '/.*/'; fail with:
Warning: [mysql error 1064] You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'at Nameless.JPG','Kamen' at Nameless','2014-01-22 23:45:16','./galleries/Photo/' at line 11555
Additional Informationpiwigo/admin/site_update.php:512 use filename without pwg_db_real_escape_string, like in piwigo/admin/include/functions_upload.inc.php:354
TagsNo tags attached.
browserany
Database engine and version
PHP version
Web serverApache 1.3.x
Attached Files

- Relationships
duplicate of 0002762assignedplg Using the ' (aphostrophe) character in filenames/directory names let's sql queries fail 

-  Notes
(0007332)
JanisV (reporter)
2014.01.23 12:13

Related issue 0002971
(0007637)
bryanw (reporter)
2014.09.25 22:47

It is crashing because you modified the regex from the default. Set it back to the default value of: /^[a-zA-Z0-9-_.]+$/

That is, the system only accepts a range of the English a-z and A-Z, the numbers 0-9, the underscore, the single dash, and the dot/period. No other characters are allowed. No spaces. No accents like é. No slashes (forward or back). No symbols. Using them can cause the app to crash or the database to become corrupt or other bad things to happen.

Unfortunately bad handling of input is pervasive through the Piwigo code.

Apostrophe, accented characters, slashes, and many other characters cause serious problems and the dev team does not want to devote the resources to rewrite large segments of the code. Remember it is a small team of volunteers.

In addition to file names it also impacts email addresses, user comments, metadata, and more.

Since the dev team is small I've looked into fixing it and posting a patch (I'm a programmer by day) but the extent of the change is huge. It impacts almost every part of the program. Nearly everything in the system touches the database, and nearly all of the database code improperly handles queries. It is not a simple bug fix, but a near-complete rewrite of the entire database interface.

From a correctness point of view this is a serious bug. Any character should be valid. Any file name supported by the filesystem should be valid. Any character allowed by the RFC should be valid for email. Special characters that are exposed as URLs can be escaped, and those accepted from forms can be properly handled and sanitized. Sadly the code does not properly handle user input and file names as distinct objects, but mixes it in with other code.
(0007653)
plg (manager)
2014.10.01 16:41

At the beginning, we have set restrictions on file/directory names for synchronization. The rest of the code was relying on these restrictions and we can't say it's a bug to consider there is no quote in file path. Then, under heavy pressure :-), we have added a configuration parameter so that webmasters can extend the authorized characters. Now someone found that using a specific character breaks some piece of code. I'm sorry but I don't think it's prioritary to be compatible with any character.

By the way, appart from synchronization, if you use any other method, there is no problem with quotes or chinese character or whatever. The problem is only for synchronization. I don't know if it really worth it to make sync compatible with "any" character.

The bugtracker is not the right place for "discussions". The forum is the better place :-)
(0007656)
bryanw (reporter)
2014.10.01 20:58
edited on: 2014.10.01 21:03

You're right, as you have replied to quite a few forum comments discounting the bug. Yes, the developers have known about the issue for at least 11 years (e.g. http://piwigo.org/forum/viewtopic.php?id=957 [^] is probably the oldest) the issue is all over the forums, especially the French forums where I see about 400 threads on the topic. Several of them declared it was a fatal flaw and they would look at other products.

As I wrote, I have looked through the code and agree that it is a massive change. I don't envy the dev team on how they intend to fix it.

For it not affecting other systems or being common in the code, a quick search of the bug database disagrees: 0000581 0000618 0001366 0002064 0002282 0002419 and many more.

In my view (as a full-time developer) it is a serious bug that valid file names are not permitted during file synchronization. Valid names on the OS should be valid names for ingest to the app. Even if the app renames them internally, the file names are legal according to the OS and should be synchronized.


- Issue History
Date Modified Username Field Change
2014.01.23 12:06 JanisV New Issue
2014.01.23 12:06 JanisV browser => any
2014.01.23 12:06 JanisV Web server => Apache 1.3.x
2014.01.23 12:13 JanisV Note Added: 0007332
2014.01.24 14:42 flop25 Relationship added duplicate of 0002762
2014.09.25 22:47 bryanw Note Added: 0007637
2014.10.01 16:41 plg Note Added: 0007653
2014.10.01 20:58 bryanw Note Added: 0007656
2014.10.01 21:03 bryanw Note Edited: 0007656
2015.02.10 09:39 plg Target Version => 2.8.0beta1


Copyright © 2000 - 2018 MantisBT Team
Contact
Powered by Mantis Bugtracker