Piwigo.org

hamiltont · 2015-02-19 05:13:50

Hey all,

I've read about this option, and would propose to update config_default.inc.php. If I'm misunderstanding this conf option please let me know. Also, I'm curious why it's not just enabled
all the time - is there a substantial cost to using an "action" URL?

From this

Code:

// one of '', 'images', 'all'
//TODO: Put this in admin and also manage .htaccess in #sites and upload folders
$conf['original_url_protection'] = '';

To this:

Code:

// When creating derived files from originals (such as thumbnails), should 
// Piwigo actively try to hide the URL of the full-quality original file?
// 
// options:
//    ''       - don't hide original URLs
//    'images' - hide URLs to full-sized images
//    'all'    - hide all original URLs
// 
// TODO: Put this in admin and also manage .htaccess in #sites and upload folders
$conf['original_url_protection'] = '';

Last edited by hamiltont (2015-02-19 05:14:36)

flop25 · 2015-02-19 07:48:50

Hi
well it's passing the pictures through php so the server will need to process that instead of just delivering the pics, and so cpu/memory cost
Not sure a shared hosting would appreciate that on the long term

hamiltont · 2015-02-20 00:03:20

Thanks for the answer. FYI readline shouldn't be memory intensive - here's an excerpt from the documentation

readfile() will not present any memory issues, even when sending large files, on its own. If you encounter an out of memory error ensure that output buffering is off

So the main cost (over having a web server directly return the file) is running a PHP script and hitting the database once per image. Would most galleries even notice this extra hit? I briefly looked through some galleries listed on the forum, and it seems like 50 images per page is a decent guess at a 90th percentile of all pages. Assuming 1ms extra for each call to action.php, that's at most 50ms extra per page. So I can't help but wonder if it's worth maintaining three code paths (one for each setting of original_url_protection) to shave a few ms off.

I am new to this codebase so perhaps I'm missing something though, thanks for reading :-)

H

homdax · 2015-02-20 12:32:35

If you encounter an out of memory error ensure that output buffering is off

Do you have any experience in managing sites on cheap or free hosts, where the webmaster can not do anything at all in regards to PHP, let alone memory allocation?

Sure you may be able to use a .htaccess hack to increase PHP memory, or even place another php.ini as a possible workaround, but for most hosts this is not allowed, even if you are technically able to do it anyway. If they notice they may cancel accounts.

Now, I am not a programmer, but whenever you try to implement new functions one has to take above in to consideration. I can not say what is the default conf for most hosts where you can not legally alter PHP execution and/or memory allocation, but I assume that a low performance hit in any changes is a better solution than telling a Piwigo user "it is your host settings, ask them to fix it" because many will not be able to.

Last edited by homdax (2015-02-20 12:33:47)

hamiltont · 2015-02-21 05:59:46

Do you have any experience in managing sites on cheap or free hosts, where the webmaster can not do anything at all in regards to PHP, let alone memory allocation?

I do, and AFAIK output buffering doesn't fall into the category of "the admin sets it and I can't change it" - it's more like a "there is a default but you're free to modify as needed" type of setting. It *can* be set in the php.ini file, but it can be turned on and off at any time e.g. Piwigo could easily disable output buffering inside of action.php directly before calling readlines to ensure this wasn't a high-memory operation. I'm open to being shown a host where this in incorrect, but my experience tells me output buffering isn't a sensitive option that's strictly controlled by the engine and scripts are 100% allowed to enable and disable it as needed.

whenever you try to implement new functions one has to take above in to consideration

Perhaps I was unclear above - I'm advocating removing old functions, not adding new ones...

I'm saying that there are currently three unique configurations being maintained in the piwigo codebase for this option, and from my analysis we could reduce it to one unique configuration, with a worst-case cost of 50ms per page load on specially-configured Piwigo installations. I'm the kind of programmer that's always looking to reduce complexity, as I think it leads to better maintained code with a smaller attack surface.

mistic100 · 2015-02-21 09:25:36

Output buffering is of course not a problem. ob_* are AFAIK always available.

Personally I'm for this change, but some people in the team want to gain every execution millisecond possible.

There is one problem though, I encountered with Batch Downloader (where I use a modified version of readfile, there is a bug in this function is certain PHP version), a few users have been banned from their hosts because "PHP is not for downloading files" (s**ty hosts if you ask me).

BUT these users are in very minority and I don't know if these hosts will complain for files much smaller.

homdax · 2015-02-21 15:55:07

hamiltont wrote:
I'm the kind of programmer that's always looking to reduce complexity, as I think it leads to better maintained code with a smaller attack surface.

Kudos for that. The programmers I have "managed" were always told to make an interface for everything that could even remotely require user input. A webmaster changing code manually is a user potentially ruining his site. No exceptions.

hamiltont · 2015-02-21 18:29:32

There is one problem though, I encountered with Batch Downloader (where I use a modified version of readfile, there is a bug in this function is certain PHP version), a few users have been banned from their hosts because "PHP is not for downloading files" (s**ty hosts if you ask me).

Ouch, although it's probably worth realizing that this change would be best if we also extracted the main "meat" of action.php (e..g the part that hides the real URL) into one of the function_* files, so that plugins could directly access the file path. The original path would still exist and be available from the web, so stuff like BatchDownloader could get the original paths and send those to the client, instead of sending the client a ton of action.php links

Piwigo.org

Announcement

#1 2015-02-19 05:13:50

Minor updates to description of conf['original_url_protection']

Code:

Code:

#2 2015-02-19 07:48:50

Re: Minor updates to description of conf['original_url_protection']

#3 2015-02-20 00:03:20

Re: Minor updates to description of conf['original_url_protection']

#4 2015-02-20 12:32:35

Re: Minor updates to description of conf['original_url_protection']

#5 2015-02-21 05:59:46

Re: Minor updates to description of conf['original_url_protection']

#6 2015-02-21 09:25:36

Re: Minor updates to description of conf['original_url_protection']

#7 2015-02-21 15:55:07

Re: Minor updates to description of conf['original_url_protection']

hamiltont wrote:

#8 2015-02-21 18:29:32

Re: Minor updates to description of conf['original_url_protection']

Board footer