[
https://issues.apache.org/jira/browse/OFBIZ-9777?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Julian Leichert updated OFBIZ-9777:
-----------------------------------
Attachment:
OFBIZ-9777_org.apache.ofbiz.product.imagemanagement_bugfixes.patch
class ScaleImage
- multiple lines : changed "" to '' in indexOf to increase performance
- multiple lines : added locale
- line 139 : changed type to viewType, type is always null in this case and i
think viewType was
intended
- line 204 : added if to handle return value of file.delete()
class CropImage
- line 71,82 : removed dls
- line 105 : indexOf "" to ''
class FrameImage
- line 70 : removed dls
- multiple lines : indexOf "" to ''
- line 379,444 : handling return value of file.delete()
- multiple lines : removed redundant toString
- line 244f : changed int to float
class ImageManagementServices
- multiple lines : changed "" to '' in indexOf to increase performance
- multiple lines : added locale
- multiple lines : removed dls
- line 87 : useless object removed
- line 109 : removed dls
- line 122,136,144 : removed now useless block of code (because of line 87)
- line 149 : added else to prevent NPE
- line 190 : removed (useless)
- line 386,762,844 : handling return value of delete()
- line 549 : removed useless Object
- line 868 : added multicatch
class ImageUrlServlet
- removed useless Overrides
class ReplaceImage
- added multicatch
class RotateImage
- multiple lines : removed dls
- line 108 : int to float
> [FB] Package org.apache.ofbiz.product.imagemanagement
> -----------------------------------------------------
>
> Key: OFBIZ-9777
> URL: https://issues.apache.org/jira/browse/OFBIZ-9777
> Project: OFBiz
> Issue Type: Sub-task
> Components: product
> Affects Versions: Trunk
> Reporter: Julian Leichert
> Priority: Minor
> Attachments:
> OFBIZ-9777_org.apache.ofbiz.product.imagemanagement_bugfixes.patch
>
>
> CropImage.java:71, DLS_DEAD_LOCAL_STORE
> DLS: Dead store to contentResult in
> org.apache.ofbiz.product.imagemanagement.CropImage.imageCrop(DispatchContext,
> Map)
> This instruction assigns a value to a local variable, but the value is not
> read or used in any subsequent instruction. Often, this indicates an error,
> because the value computed is never used.
> Note that Sun's javac compiler often generates dead stores for final local
> variables. Because FindBugs is a bytecode-based tool, there is no easy way to
> eliminate these false positives.
> CropImage.java:82, DLS_DEAD_LOCAL_STORE
> - DLS: Dead store to contentThumbResult in
> org.apache.ofbiz.product.imagemanagement.CropImage.imageCrop(DispatchContext,
> Map)
> This instruction assigns a value to a local variable, but the value is not
> read or used in any subsequent instruction. Often, this indicates an error,
> because the value computed is never used.
> Note that Sun's javac compiler often generates dead stores for final local
> variables. Because FindBugs is a bytecode-based tool, there is no easy way to
> eliminate these false positives.
> FrameImage.java:70, DLS_DEAD_LOCAL_STORE
> - DLS: Dead store to result in
> org.apache.ofbiz.product.imagemanagement.FrameImage.addImageFrame(DispatchContext,
> Map)
> This instruction assigns a value to a local variable, but the value is not
> read or used in any subsequent instruction. Often, this indicates an error,
> because the value computed is never used.
> Note that Sun's javac compiler often generates dead stores for final local
> variables. Because FindBugs is a bytecode-based tool, there is no easy way to
> eliminate these false positives.
> FrameImage.java:237, ICAST_IDIV_CAST_TO_DOUBLE
> - ICAST: Integral division result cast to double or float in
> org.apache.ofbiz.product.imagemanagement.FrameImage.combineBufferedImage(Image,
> Image, int)
> This code casts the result of an integral division (e.g., int or long
> division) operation to double or float. Doing division on integers truncates
> the result to the integer value closest to zero. The fact that the result was
> cast to double suggests that this precision should have been retained. What
> was probably meant was to cast one or both of the operands to double before
> performing the division. Here is an example:
> int x = 2;
> int y = 5;
> // Wrong: yields result 0.0
> double value1 = x / y;
> // Right: yields result 0.4
> double value2 = x / (double) y;
> FrameImage.java:266, DM_STRING_TOSTRING
> - Dm:
> org.apache.ofbiz.product.imagemanagement.FrameImage.uploadFrame(HttpServletRequest,
> HttpServletResponse) invokes toString() method on a String
> Calling String.toString() is just a redundant operation. Just use the String.
> FrameImage.java:379, RV_RETURN_VALUE_IGNORED_BAD_PRACTICE
> - RV: Exceptional return value of java.io.File.delete() ignored in
> org.apache.ofbiz.product.imagemanagement.FrameImage.previewFrameImage(HttpServletRequest,
> HttpServletResponse)
> This method returns a value that is not checked. The return value should be
> checked since it can indicate an unusual or unexpected function execution.
> For example, the File.delete() method returns false if the file could not be
> successfully deleted (rather than throwing an Exception). If you don't check
> the result, you won't notice if the method invocation signals unexpected
> behavior by returning an atypical return value.
> FrameImage.java:381, PT_RELATIVE_PATH_TRAVERSAL
> - PT: Relative path traversal in
> org.apache.ofbiz.product.imagemanagement.FrameImage.previewFrameImage(HttpServletRequest,
> HttpServletResponse)
> The software uses an HTTP request parameter to construct a pathname that
> should be within a restricted directory, but it does not properly neutralize
> sequences such as ".." that can resolve to a location that is outside of that
> directory. See http://cwe.mitre.org/data/definitions/23.html for more
> information.
> FindBugs looks only for the most blatant, obvious cases of relative path
> traversal. If FindBugs found any, you almost certainly have more
> vulnerabilities that FindBugs doesn't report. If you are concerned about
> relative path traversal, you should seriously consider using a commercial
> static analysis or pen-testing tool.
> FrameImage.java:413, DM_STRING_TOSTRING
> - Dm:
> org.apache.ofbiz.product.imagemanagement.FrameImage.chooseFrameImage(HttpServletRequest,
> HttpServletResponse) invokes toString() method on a String
> Calling String.toString() is just a redundant operation. Just use the String.
> FrameImage.java:444, RV_RETURN_VALUE_IGNORED_BAD_PRACTICE
> - RV: Exceptional return value of java.io.File.delete() ignored in
> org.apache.ofbiz.product.imagemanagement.FrameImage.deleteFrameImage(HttpServletRequest,
> HttpServletResponse)
> This method returns a value that is not checked. The return value should be
> checked since it can indicate an unusual or unexpected function execution.
> For example, the File.delete() method returns false if the file could not be
> successfully deleted (rather than throwing an Exception). If you don't check
> the result, you won't notice if the method invocation signals unexpected
> behavior by returning an atypical return value.
> ImageManagementServices.java:108, DLS_DEAD_LOCAL_STORE
> - DLS: Dead store to contentResult in
> org.apache.ofbiz.product.imagemanagement.ImageManagementServices.addMultipleuploadForProduct(DispatchContext,
> Map)
> This instruction assigns a value to a local variable, but the value is not
> read or used in any subsequent instruction. Often, this indicates an error,
> because the value computed is never used.
> Note that Sun's javac compiler often generates dead stores for final local
> variables. Because FindBugs is a bytecode-based tool, there is no easy way to
> eliminate these false positives.
> ImageManagementServices.java:135, DLS_DEAD_LOCAL_STORE
> - DLS: Dead store to fileExtension in
> org.apache.ofbiz.product.imagemanagement.ImageManagementServices.addMultipleuploadForProduct(DispatchContext,
> Map)
> This instruction assigns a value to a local variable, but the value is not
> read or used in any subsequent instruction. Often, this indicates an error,
> because the value computed is never used.
> Note that Sun's javac compiler often generates dead stores for final local
> variables. Because FindBugs is a bytecode-based tool, there is no easy way to
> eliminate these false positives.
> ImageManagementServices.java:145, DLS_DEAD_LOCAL_STORE
> - DLS: Dead store to filenameToUse in
> org.apache.ofbiz.product.imagemanagement.ImageManagementServices.addMultipleuploadForProduct(DispatchContext,
> Map)
> This instruction assigns a value to a local variable, but the value is not
> read or used in any subsequent instruction. Often, this indicates an error,
> because the value computed is never used.
> Note that Sun's javac compiler often generates dead stores for final local
> variables. Because FindBugs is a bytecode-based tool, there is no easy way to
> eliminate these false positives.
> ImageManagementServices.java:189, DLS_DEAD_LOCAL_STORE
> - DLS: Dead store to uploadFileName in
> org.apache.ofbiz.product.imagemanagement.ImageManagementServices.addMultipleuploadForProduct(DispatchContext,
> Map)
> This instruction assigns a value to a local variable, but the value is not
> read or used in any subsequent instruction. Often, this indicates an error,
> because the value computed is never used.
> Note that Sun's javac compiler often generates dead stores for final local
> variables. Because FindBugs is a bytecode-based tool, there is no easy way to
> eliminate these false positives.
> ImageManagementServices.java:219, NP_NULL_PARAM_DEREF
> - NP: Null passed for nonnull parameter of
> createContentThumbnail(DispatchContext, Map, GenericValue, ByteBuffer,
> String, String) in
> org.apache.ofbiz.product.imagemanagement.ImageManagementServices.addMultipleuploadForProduct(DispatchContext,
> Map)
> This method call passes a null value for a non-null method parameter. Either
> the parameter is annotated as a parameter that should always be non-null, or
> analysis has shown that it will always be dereferenced.
> ImageManagementServices.java:293, RV_RETURN_VALUE_IGNORED_BAD_PRACTICE
> - RV: Exceptional return value of java.io.File.delete() ignored in
> org.apache.ofbiz.product.imagemanagement.ImageManagementServices.removeImageFileForImageManagement(DispatchContext,
> Map)
> This method returns a value that is not checked. The return value should be
> checked since it can indicate an unusual or unexpected function execution.
> For example, the File.delete() method returns false if the file could not be
> successfully deleted (rather than throwing an Exception). If you don't check
> the result, you won't notice if the method invocation signals unexpected
> behavior by returning an atypical return value.
> ImageManagementServices.java:384, RV_RETURN_VALUE_IGNORED_BAD_PRACTICE
> - RV: Exceptional return value of java.io.File.delete() ignored in
> org.apache.ofbiz.product.imagemanagement.ImageManagementServices.scaleImageMangementInAllSize(DispatchContext,
> Map, String, String, String)
> This method returns a value that is not checked. The return value should be
> checked since it can indicate an unusual or unexpected function execution.
> For example, the File.delete() method returns false if the file could not be
> successfully deleted (rather than throwing an Exception). If you don't check
> the result, you won't notice if the method invocation signals unexpected
> behavior by returning an atypical return value.
> ImageManagementServices.java:431, DLS_DEAD_LOCAL_STORE
> - DLS: Dead store to dataResourceResult in
> org.apache.ofbiz.product.imagemanagement.ImageManagementServices.createContentAndDataResource(DispatchContext,
> GenericValue, String, String, String, String)
> This instruction assigns a value to a local variable, but the value is not
> read or used in any subsequent instruction. Often, this indicates an error,
> because the value computed is never used.
> Note that Sun's javac compiler often generates dead stores for final local
> variables. Because FindBugs is a bytecode-based tool, there is no easy way to
> eliminate these false positives.
> ImageManagementServices.java:497, DLS_DEAD_LOCAL_STORE
> - DLS: Dead store to contentThumbResult in
> org.apache.ofbiz.product.imagemanagement.ImageManagementServices.createContentThumbnail(DispatchContext,
> Map, GenericValue, ByteBuffer, String, String)
> This instruction assigns a value to a local variable, but the value is not
> read or used in any subsequent instruction. Often, this indicates an error,
> because the value computed is never used.
> Note that Sun's javac compiler often generates dead stores for final local
> variables. Because FindBugs is a bytecode-based tool, there is no easy way to
> eliminate these false positives.
> ImageManagementServices.java:515, DLS_DEAD_LOCAL_STORE
> - DLS: Dead store to fileExtensionThumb in
> org.apache.ofbiz.product.imagemanagement.ImageManagementServices.createContentThumbnail(DispatchContext,
> Map, GenericValue, ByteBuffer, String, String)
> This instruction assigns a value to a local variable, but the value is not
> read or used in any subsequent instruction. Often, this indicates an error,
> because the value computed is never used.
> Note that Sun's javac compiler often generates dead stores for final local
> variables. Because FindBugs is a bytecode-based tool, there is no easy way to
> eliminate these false positives.
> ImageManagementServices.java:700, DLS_DEAD_LOCAL_STORE
> - DLS: Dead store to contentThumbResult in
> org.apache.ofbiz.product.imagemanagement.ImageManagementServices.createNewImageThumbnail(DispatchContext,
> Map)
> This instruction assigns a value to a local variable, but the value is not
> read or used in any subsequent instruction. Often, this indicates an error,
> because the value computed is never used.
> Note that Sun's javac compiler often generates dead stores for final local
> variables. Because FindBugs is a bytecode-based tool, there is no easy way to
> eliminate these false positives.
> ImageManagementServices.java:725, REC_CATCH_EXCEPTION
> - REC: Exception is caught when Exception is not thrown in
> org.apache.ofbiz.product.imagemanagement.ImageManagementServices.createNewImageThumbnail(DispatchContext,
> Map)
> This method uses a try-catch block that catches Exception objects, but
> Exception is not thrown within the try block, and RuntimeException is not
> explicitly caught. It is a common bug pattern to say try { ... } catch
> (Exception e) { something } as a shorthand for catching a number of types of
> exception each of whose catch blocks is identical, but this construct also
> accidentally catches RuntimeException as well, masking potential bugs.
> A better approach is to either explicitly catch the specific exceptions that
> are thrown, or to explicitly catch RuntimeException exception, rethrow it,
> and then catch all non-Runtime Exceptions, as shown below:
> try {
> ...
> } catch (RuntimeException e) {
> throw e;
> } catch (Exception e) {
> ... deal with all non-runtime exceptions ...
> }
> ImageManagementServices.java:783, RV_RETURN_VALUE_IGNORED_BAD_PRACTICE
> - RV: Exceptional return value of java.io.File.delete() ignored in
> org.apache.ofbiz.product.imagemanagement.ImageManagementServices.renameImage(DispatchContext,
> Map)
> This method returns a value that is not checked. The return value should be
> checked since it can indicate an unusual or unexpected function execution.
> For example, the File.delete() method returns false if the file could not be
> successfully deleted (rather than throwing an Exception). If you don't check
> the result, you won't notice if the method invocation signals unexpected
> behavior by returning an atypical return value.
> ImageManagementServices.java:887, REC_CATCH_EXCEPTION
> - REC: Exception is caught when Exception is not thrown in
> org.apache.ofbiz.product.imagemanagement.ImageManagementServices.renameImage(DispatchContext,
> Map)
> This method uses a try-catch block that catches Exception objects, but
> Exception is not thrown within the try block, and RuntimeException is not
> explicitly caught. It is a common bug pattern to say try { ... } catch
> (Exception e) { something } as a shorthand for catching a number of types of
> exception each of whose catch blocks is identical, but this construct also
> accidentally catches RuntimeException as well, masking potential bugs.
> A better approach is to either explicitly catch the specific exceptions that
> are thrown, or to explicitly catch RuntimeException exception, rethrow it,
> and then catch all non-Runtime Exceptions, as shown below:
> try {
> ...
> } catch (RuntimeException e) {
> throw e;
> } catch (Exception e) {
> ... deal with all non-runtime exceptions ...
> }
> ImageUrlServlet.java:45, SE_NO_SERIALVERSIONID
> - SnVI: org.apache.ofbiz.product.imagemanagement.ImageUrlServlet is
> Serializable; consider declaring a serialVersionUID
> This class implements the Serializable interface, but does not define a
> serialVersionUID field. A change as simple as adding a reference to a .class
> object will add synthetic fields to the class, which will unfortunately
> change the implicit serialVersionUID (e.g., adding a reference to
> String.class will generate a static field class$java$lang$String). Also,
> different source code to bytecode compilers may use different naming
> conventions for synthetic variables generated for references to class objects
> or inner classes. To ensure interoperability of Serializable across versions,
> consider adding an explicit serialVersionUID.
> ReplaceImage.java:123, REC_CATCH_EXCEPTION
> - REC: Exception is caught when Exception is not thrown in
> org.apache.ofbiz.product.imagemanagement.ReplaceImage.replaceImageToExistImage(DispatchContext,
> Map)
> This method uses a try-catch block that catches Exception objects, but
> Exception is not thrown within the try block, and RuntimeException is not
> explicitly caught. It is a common bug pattern to say try { ... } catch
> (Exception e) { something } as a shorthand for catching a number of types of
> exception each of whose catch blocks is identical, but this construct also
> accidentally catches RuntimeException as well, masking potential bugs.
> A better approach is to either explicitly catch the specific exceptions that
> are thrown, or to explicitly catch RuntimeException exception, rethrow it,
> and then catch all non-Runtime Exceptions, as shown below:
> try {
> ...
> } catch (RuntimeException e) {
> throw e;
> } catch (Exception e) {
> ... deal with all non-runtime exceptions ...
> }
> RotateImage.java:69, DLS_DEAD_LOCAL_STORE
> - DLS: Dead store to contentResult in
> org.apache.ofbiz.product.imagemanagement.RotateImage.imageRotate(DispatchContext,
> Map)
> This instruction assigns a value to a local variable, but the value is not
> read or used in any subsequent instruction. Often, this indicates an error,
> because the value computed is never used.
> Note that Sun's javac compiler often generates dead stores for final local
> variables. Because FindBugs is a bytecode-based tool, there is no easy way to
> eliminate these false positives.
> RotateImage.java:80, DLS_DEAD_LOCAL_STORE
> - DLS: Dead store to contentThumbResult in
> org.apache.ofbiz.product.imagemanagement.RotateImage.imageRotate(DispatchContext,
> Map)
> This instruction assigns a value to a local variable, but the value is not
> read or used in any subsequent instruction. Often, this indicates an error,
> because the value computed is never used.
> Note that Sun's javac compiler often generates dead stores for final local
> variables. Because FindBugs is a bytecode-based tool, there is no easy way to
> eliminate these false positives.
> RotateImage.java:108, ICAST_IDIV_CAST_TO_DOUBLE
> - ICAST: Integral division result cast to double or float in
> org.apache.ofbiz.product.imagemanagement.RotateImage.imageRotate(DispatchContext,
> Map)
> This code casts the result of an integral division (e.g., int or long
> division) operation to double or float. Doing division on integers truncates
> the result to the integer value closest to zero. The fact that the result was
> cast to double suggests that this precision should have been retained. What
> was probably meant was to cast one or both of the operands to double before
> performing the division. Here is an example:
> int x = 2;
> int y = 5;
> // Wrong: yields result 0.0
> double value1 = x / y;
> // Right: yields result 0.4
> double value2 = x / (double) y;
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
