Minor errors in 5 Beta 25

[lpatrick]

Code: Select all

<?xml version="1.0" encoding="utf-8"?>
<products>
	<product>
		<title>Images (7777)</title>
		<alias>image-7777</alias>
		<sku></sku>
		<price>3</price>
		<price_original>0.0000</price_original>
		<categories>
			<category>
				<id>8:test-images</id>
				<ordering>1</ordering>
			</category>
		</categories>
		<access>1</access>
		<groups>
			<group>
				<id>1</id>
				<title>COM_PHOCACART_DEFAULT</title>
			</group>
		</groups>
		<featured>0</featured>
		<condition>0</condition>
		<image>test-images/image-7777.jpg</image>
		<attributes>
			<attribute>
				<id>0</id>
				<title>Size</title>
				<alias>size</alias>
				<required>1</required>
				<type>1</type>
				<options>
					<option>
						<title>Medium 1500 px</title>
						<alias>medium</alias>
						<amount>0.0000</amount>
						<operator>+</operator>
						<stock>0</stock>
						<operator_weight>+</operator_weight>
						<weight>0.0000</weight>
						<image></image>
						<image_medium></image_medium>
						<image_small></image_small>
						<download_folder>AZFCoH3du2OQ8biW7/m</download_folder>
						<download_token></download_token>
						<download_file>AZFCoH3du2OQ8biW7/m/image-7777m.jpg</download_file>
						<color></color>
						<default_value></default_value>
					</option>
					<option>
						<title>Large 2500 px</title>
						<alias>large</alias>
						<amount>1</amount>
						<operator>+</operator>
						<stock>0</stock>
						<operator_weight>+</operator_weight>
						<weight>0.0000</weight>
						<image></image>
						<image_medium></image_medium>
						<image_small></image_small>
						<download_folder>AZFCoH3du2OQ8biW7/l</download_folder>
						<download_token></download_token>
						<download_file>AZFCoH3du2OQ8biW7/l/image-7777l.jpg</download_file>
						<color></color>
						<default_value></default_value>
					</option>
					<option>
						<title>Small 500 px</title>
						<alias>small</alias>
						<amount>1</amount>
						<operator>-</operator>
						<stock>0</stock>
						<operator_weight>+</operator_weight>
						<weight>0.0000</weight>
						<image></image>
						<image_medium></image_medium>
						<image_small></image_small>
						<download_folder>AZFCoH3du2OQ8biW7/s</download_folder>
						<download_token></download_token>
						<download_file>AZFCoH3du2OQ8biW7/s/image-7777s.jpg</download_file>
						<color></color>
						<default_value></default_value>
					</option>
				</options>
			</attribute>
		</attributes>
		<stock>0</stock>
		<stock_calculation>0</stock_calculation>
		<stockstatus_a_id>2</stockstatus_a_id>
		<stockstatus_n_id>1</stockstatus_n_id>
		<min_quantity>0</min_quantity>
		<min_multiple_quantity>0</min_multiple_quantity>
		<min_quantity_calculation>0</min_quantity_calculation>
		<delivery_date>0000-00-00 00:00:00</delivery_date>
		<type>1</type>
		<points_needed>0</points_needed>
		<points_received>0</points_received>
		<Published>1</Published>
		<language>*</language>
		<date_update>0000-00-00 00:00:00</date_update>
		<created>2024-02-29</created>
		<modified>2024-02-29</modified>
		<metadata>{"robots":""}</metadata>
		<sales>0</sales>
	</product>
</products>

[Jan]

Hi, for file download in security tokens, the folders are created automatically and the system counts with them. It is not possible to create own token folders, as they are ready for each product or attribute. This is not a folder for images or thumbnails to display for for download. So e.g. the best it to store there zip package without any subfolders, etc.

Anyway, testing now - with different subfolders like in your example and still when deleting product, only the product folder will be deleted

Jan

[lpatrick]

Jan
I am aware that I may not be using the Import function for what it was originally created.
But if Export/Import serve to transfer data to another/new Phoca Cart site, then I don't see the difference between generating a (correct) CSV/XML ourselves. There the tokens wouldn't be generated by the new Phoca Cart site either.
And in the list of main features of Phoca Cart, there is:
- Exporting of products to XML or CSV format
- Importing of products from XML or CSV format
Which is one of the reasons I'm proposing this as a solution to my customer.

Could you please bear with me and point out what would be wrong in the XML?

It isn't possible to ask my customer to import a sql file which I would create for him whenever he has a new gallery to upload. Moreover, I didn't find a DB description yet. Wouldn't you feel much less inclined to support me if I went and 'hacked' your DB with queries whilst perhaps forgetting dependencies?

For me, except for this deletion of the folder, all seems to function well and I think it's awesome functionality to have.

Or are you saying that each product MUST have it's own subfolder? And even a different subfolder for each Attribute image?

I will go see if I can find the code where you delete the folder and perhaps hack it so that, when the folder still has (other) content AFTER deleting the product (attribute/download) file, it doesn't remove the folder.

Thanks for considering and for all the time and effort you put in this.

P.S. It's not normal that Phoca Cart can't be found in the Joomla Extensions (with the search)

[lpatrick]

So I did these edits: (see again next post)

===========================================================
1. They no longer remove the main folder
2. However, my files aren't being deleted although the code says they are. I debugged like this in file.php
if(Folder::exists($path['orig_abs_ds'] . $v['download_folder'])) {
Factory::getApplication()->enqueueMessage('Deleting: ' .$path['orig_abs_ds'] . $v['download_file'], 'success');
if (File::exists($pathAttributes['orig_abs_ds'] . $v['download_file'])) {
// File::delete($pathAttributes['orig_abs_ds'] . $v['download_file']);
unlink($pathAttributes['orig_abs_ds'] . $v['download_file']);
}
if (File::exists($pathAttributes['orig_abs_ds'] . $v['download_file'])) {
Factory::getApplication()->enqueueMessage($path['orig_abs_ds'] . $v['download_file'].' not deleted', 'error');
} else {
Factory::getApplication()->enqueueMessage($path['orig_abs_ds'] . $v['download_file'].' deleted', 'success');
}
When I run this, I get these messages:
Deleting: /var/www/html/joomla/phocacartdownload/
/var/www/html/joomla/phocacartdownload/ deleted
Deleting: /var/www/html/joomla/phocacartdownload/AZFCoH3du2OQ8biW7/m/image-7777m.jpg
/var/www/html/joomla/phocacartdownload/AZFCoH3du2OQ8biW7/m/image-7777m.jpg deleted
Deleting: /var/www/html/joomla/phocacartdownload/AZFCoH3du2OQ8biW7/l/image-7777l.jpg
/var/www/html/joomla/phocacartdownload/AZFCoH3du2OQ8biW7/l/image-7777l.jpg deleted
Deleting: /var/www/html/joomla/phocacartdownload/AZFCoH3du2OQ8biW7/s/image-7777s.jpg
/var/www/html/joomla/phocacartdownload/AZFCoH3du2OQ8biW7/s/image-7777s.jpg deleted

So I inserted that second File::exists to verify if the file is properly deleted and it confirmed it is but it's still on disk. What am I missing here?
Do I still need to do some Purge or delete Trash before the files are really deleted? I don't know enough of Joomla and couldn't find it.

3. As you can see in the messages, it did 'try' to delete phocacartdownload but my code stopped it this time. I traced the reason: I don't have anything in <download_folder> in my XML file to upload. You possibly opened the product you uploaded via my XML and I guess you saved it, which effectively filled in the non existing Download Folder automatically.

So I could add that in the XML and case closed. In that case you wouldn't need to modify your code.
But then we will need a UNIQUE folder for every image customer uploads.
On the previous site, they already had 25 galleries. Say 1000 images each x 3 sizes per image = 75000 subfolders in phocacartdownload.
Not impossible on Linux but I don't like it and in view of the fact that there's also no need for it IF you close the security on the folder, I'm really sort of hoping you might change your mind

[lpatrick]

These are the changes to attribute.php, file.php and download.php

/administrator/components/com_phocacart/libraries/phocacart/attribute# diff attribute.php attribute.php.org
606c606
< // DO NOT: Folder will remove the file(s) too
---
> // Folder will remove the file(s) too
608,614c608
< if (File:exists($pathAttributes['orig_abs_ds'] . $vF['download_file'])) {
< File::delete($pathAttributes['orig_abs_ds'] . $vF['download_file']);
< }
< // Check if folder empty before removing it
< if (!(new \FilesystemIterator($path['orig_abs_ds'] . $vF['download_folder']))->valid()) {
< Folder::delete($pathAttributes['orig_abs_ds'] . $vF['download_folder']);
< }
---
> Folder::delete($pathAttributes['orig_abs_ds'] . $vF['download_folder']);


/administrator/components/com_phocacart/libraries/phocacart/file# diff file.php file.php.org
250,256c250,251
< if(Folder::exists($path['orig_abs_ds'] . $v['download_folder'])) {
< if (File::exists($pathAttributes['orig_abs_ds'] . $v['download_file'])) {
< File::delete($pathAttributes['orig_abs_ds'] . $v['download_file']);
< }
< // Check if folder empty before removing it
< if (!(new \FilesystemIterator($path['orig_abs_ds'] . $v['download_folder']))->valid()) {
< if(Folder::delete($path['orig_abs_ds'] . $v['download_folder'])) {
---
> if(Folder::exists($path['orig_abs_ds'] . $v)) {
> if(Folder::delete($path['orig_abs_ds'] . $v)) {
258,262c253,254
< if ($manager == 'attributefile') {
< Factory::getApplication()->enqueueMessage(Text::_('COM_PHOCACART_SUCCESS_ATTRIBUTE_OPTION_DOWNLOAD_FOLDER_DELETED'). ': ' . $v['download_folder'], 'success');
< } else {
< Factory::getApplication()->enqueueMessage(Text::_('COM_PHOCACART_SUCCESS_PRODUCT_DOWNLOAD_FOLDER_DELETED'). ': ' . $v['download_folder'], 'success');
< }
---
> if ($manager == 'attributefile') {
> Factory::getApplication()->enqueueMessage(Text::_('COM_PHOCACART_SUCCESS_ATTRIBUTE_OPTION_DOWNLOAD_FOLDER_DELETED'). ': ' . $v, 'success');
264,268c256,262
< if ($manager == 'attributefile') {
< Factory::getApplication()->enqueueMessage(Text::_('COM_PHOCACART_ERROR_REMOVE_ATTRIBUTE_OPTION_DOWNLOAD_FOLDER'). ': ' . $v['download_folder'], 'error');
< } else {
< Factory::getApplication()->enqueueMessage(Text::_('COM_PHOCACART_ERROR_REMOVE_PRODUCT_DOWNLOAD_FOLDER'). ': ' . $v['download_folder'], 'error');
< }
---
> Factory::getApplication()->enqueueMessage(Text::_('COM_PHOCACART_SUCCESS_PRODUCT_DOWNLOAD_FOLDER_DELETED'). ': ' . $v, 'success');
> }
> } else {
> if ($manager == 'attributefile') {
> Factory::getApplication()->enqueueMessage(Text::_('COM_PHOCACART_ERROR_REMOVE_ATTRIBUTE_OPTION_DOWNLOAD_FOLDER'). ': ' . $v, 'error');
> } else {
> Factory::getApplication()->enqueueMessage(Text::_('COM_PHOCACART_ERROR_REMOVE_PRODUCT_DOWNLOAD_FOLDER'). ': ' . $v, 'error');


/administrator/components/com_phocacart/libraries/phocacart/download# diff download.php download.php.org
514c514
< $query = ' SELECT download_folder, download_file FROM #__phocacart_products WHERE id IN ( '.$cids.' ) ORDER BY id';
---
> $query = ' SELECT download_folder FROM #__phocacart_products WHERE id IN ( '.$cids.' ) ORDER BY id';
516c516
< $folders = $db->loadAssocList();
---
> $folders = $db->loadColumn();
530c530
< $query = ' SELECT av.download_folder, av.download_file FROM #__phocacart_attribute_values AS av'
---
> $query = ' SELECT av.download_folder FROM #__phocacart_attribute_values AS av'
535c535
< $folders = $db->loadAssocList();
---
> $folders = $db->loadColumn();

[lpatrick]

Jan

Would you please consider these small edits in three files to delete the file first, then check the folder contents and only remove the folder IF it's empty?

I mean even if it shouldn't occur in a normal situation/environment, checking it's empty before deleting a folder does no harm, does it?

I once had a customer who called me saying: "Patrick, there's a message on the screen "Uhm, Houston, we have a problem" ... what does it mean?

[Jan]

Maybe I don't fully understand this post. If you remove the product, which have assigned the token, then the completely folder needs to be removed too. It does not matter if it includes files or not. The product does not exist anymore so its tokenized folder should even not exist anymore. If we left such tokenized folders on the server, this can lead to various problems. Or do I understand this post not correctly?

Jan

[lpatrick]

... It does not matter if it includes files or not. The product does not exist anymore so its tokenized folder should even not exist anymore. If we left such tokenized folders on the server, this can lead to various problems. ...

Jan

Jan

Yes, I think in your view, the tokenised folder should not ever contain files which do not belong to the product's download or attributes.
In your 'normal' situation, that would (COULD) be true.
But I looked at the getToken in utils.php and you actually just take a random 16 character sequence from an sha256'ed string, right? IMHO there is no guarantee on uniqueness. Moreover you do not check if the new token folder already existed (and perhaps run a new getToken if it does), neither on disk nor in the DB, right? You try to create the folder and ignore if it already exists, I think.
Secondly, like I said if I'm correct no other files COULD exist if you use Phoca Cart the 'normal' way. So in your use case, checking the folder is effectively empty after deleting the attribute/download file, would normally always result in an empty folder so my code would work.
If however, the shop gets more and more products (in my case several thousands), then chances of re-using a folder with above code does become a (still small but existing) risk. And removing it, might remove more than intended when deleting a product (and it's possible it won't be noticed quickly).

I already stated before that the folders don't even have to be tokenised if:
- the phocacartdownload folder is closed off from external access as it should be
- and the webserver is set to NOT allow directory listings (for which the empty index.html are a very bad solution security wise because it's security by obscurity. And I don't think you add them to the tokenised folders either, where the real value would be stored for a shop selling digital products.)
So (in time) I would do away with the tokenised folders if I were you and let the admin create and select whatever file/folder they used for uploading (which in the case of thousands of digital products is not possible interactively).
Obviously with a strong check and warning if above security measures aren't implemented. E.g. put an image with a text 'Your phocacartdownload folder is not properly secured, please read installation instructions' or something in the phocacartdownload folder itself and display it in your control panel. If it is secured, it won't be visible, right? I would also copy the file there automatically if one changes its location in the Configuration.

Besides that, I came here with a positive story: that some functions of the software (i.c. Import), if used slightly different, can be a huge advantage. Especially for large quantities of products. (I say again: several ten thousands of subfolders in one folder, holding only one file each is not very practical to say the least.)

I know it's hard to understand why someone would use the software in a way other than how it was originally intended but if it gets you more possible users and positive reviews, why not?!

In the mean time I already found another workaround which will be even more horrific to you. I upload the XML with non existing folder names keeping all my download files in one folder per gallery and in a attribute subfolder per size. Which makes it a lot easier to manage and upload.
And since you only check if the folder exists before deleting it, it won't mess with my folder structure.
Just if the customer would delete some content, it's only deleted from the DB for now.

Unless of course I convinced you of my good intentions by now.

[Jan]

Hi, thank you for clarification:

- checking existing folders - yes, this is the way to have only unique folders - I will add this into next version
- phocacartdownload - external access - yes, this is another topic where the .htaccess can be used or the folder can be moved behind public_html
- having index.html is a method which has been canceled in Joomla (there is article about it form Nick from Akeeba), so there Phoca Cart just follows Joomla rules.

several ten thousands of subfolders in one folder, holding only one file each is not very practical to say the least.

Yes, having a lot of folders/files is always not good, but in last time I solve problems where some extensions just store all possible files into one folder and there are many thousands of files in one folder and such cannot be read by any tool, because such large amount of files is not loaded in time.

So for now I am working on the check of folder unique name and I hope I will release it in next Beta version.

Thank you for your idea.

Jan

[Jan]

Hi,
tokenized folder uniqueness check is introduced in version 5.0.0 Beta 38

https://github.com/PhocaCz/PhocaCart/re ... Beta38.zip

Deleting only files not whole folders, see:
https://github.com/PhocaCz/PhocaCart/issues/220

I still need to check if there will be no possible issues, etc.

Thank you, Jan