[ZBX-5704] Allowed upload any file types for images Created: 2012 Oct 18  Updated: 2017 May 30  Resolved: 2012 Dec 10

Status: Closed
Project: ZABBIX BUGS AND ISSUES
Component/s: Frontend (F)
Affects Version/s: 2.0.4rc1
Fix Version/s: 2.0.5rc1, 2.1.0

Type: Incident report Priority: Trivial
Reporter: Oleg Egorov (Inactive) Assignee: Unassigned
Resolution: Fixed Votes: 0
Labels: images, upload
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Attachments: JPEG File issue.JPG    

 Description   

Result if upload *.exe, *.xml, *.txt...
This problem exist in create and update functions.



 Comments   
Comment by Eduards Samersovs (Inactive) [ 2012 Oct 24 ]

Fixed in development branch svn://svn.zabbix.com/branches/dev/ZBX-5704

Comment by Toms (Inactive) [ 2012 Oct 25 ]

(2) default.css there should be good reason to make changes. So padding change and color hex change is redundant.

Eduards I hope that you understood reason of adding paddings..

tomtom No, I don't know any guidelines that "padding: 0 0 0 5px;" is superior to "padding-left: 5px;" and that "background-color: #ffffff" is superior to "background-color:#FFFFFF;"

Eduards RESOLVED

tomtom CLOSED

Comment by Toms (Inactive) [ 2012 Oct 25 ]

(3) adm.images.php $confCombobox should be $confComboBox

Eduards RESOLVED, r31074

tomtom CLOSED

Comment by Toms (Inactive) [ 2012 Oct 25 ]

(4)
Here brackets were added:
public function isValid()

{ return ($this->error == UPLOAD_ERR_OK); }

Here brackets were removed:
if (!is_null($options['editable']) && self::$userData['type'] < USER_TYPE_ZABBIX_ADMIN)

{ return $result; }

Eduards Yes, if it is ternary operation we use () but in common if we use () only where it needed, for example:
if (($a > b && $b < $c) || ($a > c && $c < $b)) {}

tomtom I don't see here any ternary operation.

Eduards Any way, I don't how more precisely call this but if we have construction: return $a == $b we use () !

tomtom point me to guidelines where it is said that in "return" we use brackets and in "if statement" with to comparisions we don't!

Eduards Documented http://zabbix.org/wiki/Docs/specs/coding_style#Return_boolean_expression

tomtom CLOSED

Comment by Toms (Inactive) [ 2012 Oct 25 ]

(5) CImage.php according to sample in http://www.zabbix.org/wiki/Docs/specs/coding_style we don't put line after 'break' in 'switch' statement.

Eduards We need to discuss it, because in large switch cases it's very useful to split with empty line..

tomtom CLOSED.

Comment by Toms (Inactive) [ 2012 Oct 25 ]

(7) CImage.php line 433 why change from if "if (isset($image['image'])) {" to "(!empty($image['image'])) {"?

Eduards Because we must check if image content is empty or not..

tomtom Name example where isset() would fail. These kind of changes are very prone to errors.

Eduards Not agree at all!

tomtom this is not a case where isset() fail!

Eduards RESOLVED

tomtom CLOSED

Comment by Toms (Inactive) [ 2012 Oct 25 ]

(8) improve comments for CImage.php checkImage() method: describe all exceptions in method comment, parameter $image, magic number "10", why from $ZBX_MESSAGES are popped value.

Eduards RESOLVED, hope after this commit your question about ZBX_MESSAGE disappears. r31076

tomtom still missing comments, except magic number which were removed!

Eduards RESOLVED

tomtom CLOSED

Comment by Toms (Inactive) [ 2012 Oct 25 ]

(9) CUploadFile.php validateSize() method should be commented according to PHPDoc standart

validateSize() should be renamed to validateImageSize() as it should not be used in xml imports

Eduards RESOLVED

tomtom still comment incomplete.

Eduards RESOLVED

tomtom CLOSED

Comment by Toms (Inactive) [ 2012 Oct 25 ]

(10) in what scenario
self::exception(ZBX_API_ERROR_PARAMETERS, _s('Image "%s" has empty content.', $image['name']));
should be raised?

Eduards Why not if this is "create"??

tomtom I mean, how to get this exception in Frontend.

Eduards I have one file (.rpm) witch, I do not how, but uploads with empty content.. any way this validation extremally is necessary for API because user can not pass "image" at all or pass with empty content!

tomtom attach this file please!

Eduards RESOLVED

tomtom CLOSED

Comment by Toms (Inactive) [ 2012 Oct 25 ]

(11) I try to upload .bmp file and get error: "File format is not image. [CImage.create -> CImage.checkImage]"

Eduards Zabbix don't support bmp format, wont message "File format is not png, gif or jpg" ?

tomtom I propose to use message: "File format is unsupported".

Eduards RESOLVED, r31078

tomtom CLOSED

Comment by Toms (Inactive) [ 2012 Oct 29 ]

TESTED

Review my changes in r31117, if OK merge

Comment by Eduards Samersovs (Inactive) [ 2012 Nov 19 ]

Fixed in versions pre-2.1.0 (beta) r31502, pre-2.0.4rc1 r31503

Comment by Pavels Jelisejevs (Inactive) [ 2012 Nov 20 ]

(12) Please note this change in the API changelog.

Eduards RESOLVED

<richlv> ...for 2.0.5

jelisejev I've removed it for now, it will have to be added when merging to 2.0.5

Eduards Looks that bureaucracy procedure win bug fixing! Is it really open source? or we plan to became as microsoft...

<richlv> opensource doesn't equal "crap quality" and "i don't give a fuck about translators"

Eduards You think fix reverting improve quality?? I prefer not translated 1 string than not resolved bug!

Comment by richlv [ 2012 Nov 20 ]

(13) this was merged in a string freeze period. it added two new strings :

File format is unsupported.
Image load error [%1$s] in [%2$s].

and changed one string :

Image size must be less than 1MB -> Image size must be less than 1MB.

it must be reverted and merged again once 2.0.4 is out

Eduards REVERTED! r.31529

<richlv> CLOSED ('Image load error [%1$s] in [%2$s].' was actually added in ZBX-5737)

<richlv> actually, it was not fully reverted, incorrect changelog entry was still present in trunk changelog. removed in rev 31591.

Comment by Eduards Samersovs (Inactive) [ 2012 Dec 10 ]

Fixed in versions pre-2.0.5rc1 r32030

Generated at Thu Mar 28 19:27:26 EET 2024 using Jira 9.12.4#9120004-sha1:625303b708afdb767e17cb2838290c41888e9ff0.