Add Inventory item, Image upload failed...

[ALEXSHEN]

Hi Tim and Paul

Ok I will, real thanks for your help very much. I am sure there will be many questions to ask you, so thanks again.

Alex

[alanmi3833]

(09-09-2019 04:55 PM)falkoner Wrote:  Ah yes, I should have picked that up. Sorry!

There is a utility script called Z_ChangeStockCode.php that should change a code throughout the system. You should test this out first on a copy of your database, but it *should* work.

Alternatively if it is only the stock image that is affected then it should be possible to do some fudge with the code to save and display them. Of course then that change would need to be replicated each time you update the code, so if it woks OK then the first option is best.

Tim

On my system that works 100%, use it quite a lot.
For instance if I update an item with a new version number (eg. RS485A -> RS485B) I rather change the item code than create a new item and mark the old one obsolete.

[falkoner]

Thanks Alan, I have only ever tested it on test databases, so it's good to hear from someone who uses it on a productions system.

Tim

[TurboPT]

(09-09-2019 07:40 PM)falkoner Wrote:  Paul: We have a function called ContainsIllegalCharacters() which checks part codes for any characters that shouldn't be there (I don't really like this function but that's another story). Should we look to include the '/' in that function as a banned character?

Tim

I thought we already did, but if not, we probably should, as that character would be invalid in a file name. (clashes with path use)

I tried using touch, mv, and cp with a / in the filename, and the fail message had either "directory not found" or "file or directory not found".

[falkoner]

(09-10-2019 11:04 AM)TurboPT Wrote:  I thought we already did, but if not, we probably should, as that character would be invalid in a file name. (clashes with path use)

I tried using touch, mv, and cp with a / in the filename, and the fail message had either "directory not found" or "file or directory not found".

The function says:

if (mb_strstr($CheckVariable, "'") or mb_strstr($CheckVariable, '+') or mb_strstr($CheckVariable, '?') or mb_strstr($CheckVariable, '.') or mb_strstr($CheckVariable, "\"") or mb_strstr($CheckVariable, '&') or mb_strstr($CheckVariable, "\\") or mb_strstr($CheckVariable, '"') or mb_strstr($CheckVariable, '>') or mb_strstr($CheckVariable, '<'))

So no, it checks for '\' but not '/'

Tim

[falkoner]

I was about to commit this change, but wanted to be sure. It is used to check customer codes, supplier codes, account group names, user names, stock category codes and many more things. The full list of scripts that use it is:
AccountGroups.php
AccountSections.php
Contracts.php
CustEDISetup.php
CustLoginSetup.php
CustomerBranches.php
Customers.php
Departments.php
includes/ConnectDB.inc
install/index.php
InternalStockCategoriesByRole.php
MailingGroupMaintenance.php
PaymentMethods.php
PcExpenses.php
PcExpensesTypeTab.php
PcTypeTabs.php
StockClone.php
StockSerialItems.php
Stocks.php
Suppliers.php
SuppLoginSetup.php
TaxCategories.php
TaxProvinces.php
UnitsOfMeasure.php
WorkCentres.php
WWW_Users.php
Z_ChangeBranchCode.php
Z_ChangeGLAccountCode.php
Z_ChangeLocationCode.php
Z_ChangeSalesmanCode.php
Z_ChangeStockCategory.php
Z_ChangeStockCode.php
Z_ImportCustbranch.php
Z_ImportDebtors.php
Z_ImportStocks.php
Z_ImportSuppliers.php
Z_MakeNewCompany.php

can anybody think why not allowing a "/" character would break these scripts?

Tim

[TurboPT]

Tim, looking at the Stocks.php handling, the situation may be easier to solve than expected? (and possibly save some time and trouble messing with all the other scripts?)

The image file upload handling from lines 84 to 121 has the potential to use the $StockID as-is before checking for any illegal characters, which is not done until line 157.
Assuming no issues with the other file checks, a (potentially) bad StockID will be moved/used at line 119.

Not sure if we have this situation anywhere else, but noted this last night digging in to why only Stocks.php seemed to suddenly have this issue, brought to light by Alex's situation.

Apparently, it has been this way for awhile.
The only caveat that I can think about at the immediate moment, is the impact to existing stockID's that already have / in use.

=====

The / does not seem to be trouble for URLs, as we pass StockID around quite a bit, and this character has not been restricted, but IS a problem when it comes to file naming.
Will we need separate handling for file names vs URLs? (I hope not, but I'm open to any ideas/thoughts for best handling)

It seems that Windows has far more restrictions on file/directory characters (and naming) that can be used with Linux.

The Windows list looks more like our current illegal character set:

https://docs.microsoft.com/en-us/windows...tory-names Wrote:Use any character in the current code page for a name, including Unicode characters and characters in the extended character set (128–255), except for the following:

The following reserved characters:
< (less than)
> (greater than)
: (colon)
" (double quote)
/ (forward slash)
\ (backslash)
| (vertical bar or pipe)
? (question mark)
* (asterisk)

Integer value zero, sometimes referred to as the ASCII NUL character.

Characters whose integer representations are in the range from 1 through 31, except for alternate data streams where these characters are allowed. For more information about file streams, see File Streams.

Any other character that the target file system does not allow.

Do not use the following reserved names for the name of a file:

CON, PRN, AUX, NUL, COM1, COM2, COM3, COM4, COM5, COM6, COM7,
COM8, COM9, LPT1, LPT2, LPT3, LPT4, LPT5, LPT6, LPT7, LPT8, and
LPT9. Also avoid these names followed immediately by an extension; for example, NUL.txt is not recommended.

Linux:

https://unix.stackexchange.com/a/155836 Wrote:You can use any characters except for null and / within a filename in modern Unix and Linux filesystems.

So I'm not sure if adding the / to the current illegal character handling is best, for most of those can be used with file names, depending on the system.

[falkoner]

Hi Paul, some quick thoughts on your post (I am out for the rest of the day so can't comment more deeply till tomorrow).

The reason that Alex is the first to mention this (actually I seem to remember it came up on the mailing lists years ago but I may be wrong) is that it only affects those people who use a '/' in their part numbers (quite rare) and who also want to store images of their products (not quite so rare, but still not that common) so there won't be many people trying to do this. As it wasn't giving an error message, those that had tried may have given up.

I had thought about people with part codes already including a '/' and who don't require images of their parts. They will only be affected if they tried to add new codes. I accept that this is sub-optimal, but they could remove the change as it would be very simple to do if they wanted to carry on using '/' in their codes.

Alternatively we just provide a warning message that says something like "You have chosen a '/' character in your part code. If you do this you will be unable to store or view images of this product"

Tim