[
https://issues.apache.org/jira/browse/OFBIZ-9811?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Julian Leichert updated OFBIZ-9811:
-----------------------------------
Attachment: OFBIZ-9811_org.apache.ofbiz.content.data_bugfixes.patch
class DataEvents
- line 320 : removed dls
class DataResourceWorker
- line 179: StringBuilder to append to String
- multiple lines : "" to '' in indexOf (better performance)
- line 532, 550, 569 : added null-check
- line 572 : check on return added
- multiple lines : removed redundant null-checks
- line 963,972,983 : changed FileReader to InputStreamReader (utf8 encoding)
- line 1039 : added utf8
class DataServices
- multiple lines : "" to '' in indexOf (better performance)
- line 249,436 : try-with-ressources (simpler closing of stream) and
OutputStreamWriter (utf8 encoding)
- line 521 : conversion to String with Arrays.toString otherwise the result
wouldn't make any sense
> [FB] Package org.apache.ofbiz.content.data
> ------------------------------------------
>
> Key: OFBIZ-9811
> URL: https://issues.apache.org/jira/browse/OFBIZ-9811
> Project: OFBiz
> Issue Type: Sub-task
> Components: content
> Affects Versions: Trunk
> Reporter: Julian Leichert
> Priority: Minor
> Attachments: OFBIZ-9811_org.apache.ofbiz.content.data_bugfixes.patch
>
>
> DataEvents.java:320, DLS_DEAD_LOCAL_STORE
> - DLS: Dead store to dataResourceId in
> org.apache.ofbiz.content.data.DataEvents.persistDataResource(HttpServletRequest,
> HttpServletResponse)
> 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.
> DataResourceWorker.java:179, SBSC_USE_STRINGBUFFER_CONCATENATION
> - SBSC: org.apache.ofbiz.content.data.DataResourceWorker.buildList(Map, List,
> int) concatenates strings using + in a loop
> The method seems to be building a String using concatenation in a loop. In
> each iteration, the String is converted to a StringBuffer/StringBuilder,
> appended to, and converted back to a String. This can lead to a cost
> quadratic in the number of iterations, as the growing string is recopied in
> each iteration.
> Better performance can be obtained by using a StringBuffer (or StringBuilder
> in Java 1.5) explicitly.
> For example:
> // This is bad
> String s = "";
> for (int i = 0; i < field.length; ++i) {
> s = s + field[i];
> }
> // This is better
> StringBuffer buf = new StringBuffer();
> for (int i = 0; i < field.length; ++i) {
> buf.append(field[i]);
> }
> String s = buf.toString();
> DataResourceWorker.java:272, DM_CONVERT_CASE
> - Dm: Use of non-localized String.toUpperCase() or String.toLowerCase() in
> org.apache.ofbiz.content.data.DataResourceWorker.getMimeTypeFromImageFileName(String)
> A String is being converted to upper or lowercase, using the platform's
> default encoding. This may result in improper conversions when used with
> international characters. Use the
> String.toUpperCase( Locale l )
> String.toLowerCase( Locale l )
> versions instead.
> DataResourceWorker.java:528, NP_NULL_ON_SOME_PATH_FROM_RETURN_VALUE
> - NP: Possible null pointer dereference in
> org.apache.ofbiz.content.data.DataResourceWorker.getDataResourceContentUploadPath(String,
> double, boolean) due to return value of called method
> The return value from a method is dereferenced without a null check, and the
> return value of that method is one that should generally be checked for null.
> This may lead to a NullPointerException when the code is executed.
> DataResourceWorker.java:547, NP_NULL_ON_SOME_PATH_FROM_RETURN_VALUE
> - NP: Possible null pointer dereference in
> org.apache.ofbiz.content.data.DataResourceWorker.getDataResourceContentUploadPath(String,
> double, boolean) due to return value of called method
> The return value from a method is dereferenced without a null check, and the
> return value of that method is one that should generally be checked for null.
> This may lead to a NullPointerException when the code is executed.
> DataResourceWorker.java:555, NP_NULL_ON_SOME_PATH
> - NP: Possible null pointer dereference of latestDir in
> org.apache.ofbiz.content.data.DataResourceWorker.getDataResourceContentUploadPath(String,
> double, boolean)
> There is a branch of statement that, if executed, guarantees that a null
> value will be dereferenced, which would generate a NullPointerException when
> the code is executed. Of course, the problem might be that the branch or
> statement is infeasible and that the null pointer exception can't ever be
> executed; deciding that is beyond the ability of FindBugs.
> DataResourceWorker.java:570, RV_RETURN_VALUE_IGNORED_BAD_PRACTICE
> - RV: Exceptional return value of java.io.File.mkdir() ignored in
> org.apache.ofbiz.content.data.DataResourceWorker.makeNewDirectory(File)
> 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.
> DataResourceWorker.java:773, NP_NULL_PARAM_DEREF
> - NP: Null passed for nonnull parameter of new
> org.apache.ofbiz.widget.renderer.FormRenderer(ModelForm, FormStringRenderer)
> in
> org.apache.ofbiz.content.data.DataResourceWorker.renderDataResourceAsText(LocalDispatcher,
> Delegator, String, Appendable, Map, Locale, String, boolean, List)
> 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.
> DataResourceWorker.java:807, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE
> - RCN: Redundant nullcheck of context, which is known to be non-null in
> org.apache.ofbiz.content.data.DataResourceWorker.writeDataResourceText(GenericValue,
> String, Locale, Map, Delegator, Appendable, boolean)
> This method contains a redundant check of a known non-null value against the
> constant null.
> DataResourceWorker.java:923, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE
> - RCN: Redundant nullcheck of mimeString, which is known to be non-null in
> org.apache.ofbiz.content.data.DataResourceWorker.writeText(GenericValue,
> String, Map, String, Locale, Appendable)
> This method contains a redundant check of a known non-null value against the
> constant null.
> DataResourceWorker.java:956, DM_DEFAULT_ENCODING
> - Dm: Found reliance on default encoding in
> org.apache.ofbiz.content.data.DataResourceWorker.renderFile(String, String,
> String, Appendable): new java.io.FileReader(File)
> Found a call to a method which will perform a byte to String (or String to
> byte) conversion, and will assume that the default platform encoding is
> suitable. This will cause the application behaviour to vary between
> platforms. Use an alternative API and specify a charset name or Charset
> object explicitly.
> DataResourceWorker.java:1032, DM_DEFAULT_ENCODING
> - Dm: Found reliance on default encoding in
> org.apache.ofbiz.content.data.DataResourceWorker.getDataResourceStream(GenericValue,
> String, String, Locale, String, boolean): String.getBytes()
> Found a call to a method which will perform a byte to String (or String to
> byte) conversion, and will assume that the default platform encoding is
> suitable. This will cause the application behaviour to vary between
> platforms. Use an alternative API and specify a charset name or Charset
> object explicitly.
> DataServices.java:247, OS_OPEN_STREAM_EXCEPTION_PATH
> - OS:
> org.apache.ofbiz.content.data.DataServices.createFileMethod(DispatchContext,
> Map) may fail to close stream on exception
> The method creates an IO stream object, does not assign it to any fields,
> pass it to other methods, or return it, and does not appear to close it on
> all possible exception paths out of the method. This may result in a file
> descriptor leak. It is generally a good idea to use a finally block to
> ensure that streams are closed.
> DataServices.java:247, DM_DEFAULT_ENCODING
> - Dm: Found reliance on default encoding in
> org.apache.ofbiz.content.data.DataServices.createFileMethod(DispatchContext,
> Map): new java.io.FileWriter(File)
> Found a call to a method which will perform a byte to String (or String to
> byte) conversion, and will assume that the default platform encoding is
> suitable. This will cause the application behaviour to vary between
> platforms. Use an alternative API and specify a charset name or Charset
> object explicitly.
> DataServices.java:247, OBL_UNSATISFIED_OBLIGATION_EXCEPTION_EDGE
> - OBL:
> org.apache.ofbiz.content.data.DataServices.createFileMethod(DispatchContext,
> Map) may fail to clean up java.io.Writer on checked exception
> This method may fail to clean up (close, dispose of) a stream, database
> object, or other resource requiring an explicit cleanup operation.
> In general, if a method opens a stream or other resource, the method should
> use a try/finally block to ensure that the stream or resource is cleaned up
> before the method returns.
> This bug pattern is essentially the same as the OS_OPEN_STREAM and
> ODR_OPEN_DATABASE_RESOURCE bug patterns, but is based on a different (and
> hopefully better) static analysis technique. We are interested is getting
> feedback about the usefulness of this bug pattern. To send feedback, either:
> send email to [email protected]
> file a bug report: http://findbugs.sourceforge.net/reportingBugs.html
> In particular, the false-positive suppression heuristics for this bug pattern
> have not been extensively tuned, so reports about false positives are helpful
> to us.
> See Weimer and Necula, Finding and Preventing Run-Time Error Handling
> Mistakes, for a description of the analysis technique.
> DataServices.java:430, NP_LOAD_OF_KNOWN_NULL_VALUE
> - NP: Load of known null value in
> org.apache.ofbiz.content.data.DataServices.updateFileMethod(DispatchContext,
> Map)
> The variable referenced at this point is known to be null due to an earlier
> check against null. Although this is valid, it might be a mistake (perhaps
> you intended to refer to a different variable, or perhaps the earlier check
> to see if the variable is null should have been a check to see if it was
> non-null).
> DataServices.java:436, DM_DEFAULT_ENCODING
> - Dm: Found reliance on default encoding in
> org.apache.ofbiz.content.data.DataServices.updateFileMethod(DispatchContext,
> Map): new java.io.FileWriter(File)
> Found a call to a method which will perform a byte to String (or String to
> byte) conversion, and will assume that the default platform encoding is
> suitable. This will cause the application behaviour to vary between
> platforms. Use an alternative API and specify a charset name or Charset
> object explicitly.
> DataServices.java:436, OS_OPEN_STREAM_EXCEPTION_PATH
> - OS:
> org.apache.ofbiz.content.data.DataServices.updateFileMethod(DispatchContext,
> Map) may fail to close stream on exception
> The method creates an IO stream object, does not assign it to any fields,
> pass it to other methods, or return it, and does not appear to close it on
> all possible exception paths out of the method. This may result in a file
> descriptor leak. It is generally a good idea to use a finally block to
> ensure that streams are closed.
> DataServices.java:436, OBL_UNSATISFIED_OBLIGATION_EXCEPTION_EDGE
> - OBL:
> org.apache.ofbiz.content.data.DataServices.updateFileMethod(DispatchContext,
> Map) may fail to clean up java.io.Writer on checked exception
> This method may fail to clean up (close, dispose of) a stream, database
> object, or other resource requiring an explicit cleanup operation.
> In general, if a method opens a stream or other resource, the method should
> use a try/finally block to ensure that the stream or resource is cleaned up
> before the method returns.
> This bug pattern is essentially the same as the OS_OPEN_STREAM and
> ODR_OPEN_DATABASE_RESOURCE bug patterns, but is based on a different (and
> hopefully better) static analysis technique. We are interested is getting
> feedback about the usefulness of this bug pattern. To send feedback, either:
> send email to [email protected]
> file a bug report: http://findbugs.sourceforge.net/reportingBugs.html
> In particular, the false-positive suppression heuristics for this bug pattern
> have not been extensively tuned, so reports about false positives are helpful
> to us.
> See Weimer and Necula, Finding and Preventing Run-Time Error Handling
> Mistakes, for a description of the analysis technique.
> DataServices.java:521, DMI_INVOKING_TOSTRING_ON_ARRAY
> - USELESS_STRING: Invocation of toString on imageBytes in
> org.apache.ofbiz.content.data.DataServices.updateImageMethod(DispatchContext,
> Map)
> The code invokes toString on an array, which will generate a fairly useless
> result such as [C@16f0472. Consider using Arrays.toString to convert the
> array into a readable String that gives the contents of the array. See
> Programming Puzzlers, chapter 3, puzzle 12.
> DataServices.java:607, OS_OPEN_STREAM_EXCEPTION_PATH
> - OS:
> org.apache.ofbiz.content.data.DataServices.createBinaryFileMethod(DispatchContext,
> Map) may fail to close stream on exception
> The method creates an IO stream object, does not assign it to any fields,
> pass it to other methods, or return it, and does not appear to close it on
> all possible exception paths out of the method. This may result in a file
> descriptor leak. It is generally a good idea to use a finally block to
> ensure that streams are closed.
> DataServices.java:607, OBL_UNSATISFIED_OBLIGATION_EXCEPTION_EDGE
> - OBL:
> org.apache.ofbiz.content.data.DataServices.createBinaryFileMethod(DispatchContext,
> Map) may fail to clean up java.io.OutputStream on checked exception
> This method may fail to clean up (close, dispose of) a stream, database
> object, or other resource requiring an explicit cleanup operation.
> In general, if a method opens a stream or other resource, the method should
> use a try/finally block to ensure that the stream or resource is cleaned up
> before the method returns.
> This bug pattern is essentially the same as the OS_OPEN_STREAM and
> ODR_OPEN_DATABASE_RESOURCE bug patterns, but is based on a different (and
> hopefully better) static analysis technique. We are interested is getting
> feedback about the usefulness of this bug pattern. To send feedback, either:
> send email to [email protected]
> file a bug report: http://findbugs.sourceforge.net/reportingBugs.html
> In particular, the false-positive suppression heuristics for this bug pattern
> have not been extensively tuned, so reports about false positives are helpful
> to us.
> See Weimer and Necula, Finding and Preventing Run-Time Error Handling
> Mistakes, for a description of the analysis technique.
> DataServices.java:659, DMI_INVOKING_TOSTRING_ON_ARRAY
> - USELESS_STRING: Invocation of toString on imageData in
> org.apache.ofbiz.content.data.DataServices.updateBinaryFileMethod(DispatchContext,
> Map)
> The code invokes toString on an array, which will generate a fairly useless
> result such as [C@16f0472. Consider using Arrays.toString to convert the
> array into a readable String that gives the contents of the array. See
> Programming Puzzlers, chapter 3, puzzle 12.
> DataServices.java:663, OBL_UNSATISFIED_OBLIGATION_EXCEPTION_EDGE
> - OBL:
> org.apache.ofbiz.content.data.DataServices.updateBinaryFileMethod(DispatchContext,
> Map) may fail to clean up java.io.OutputStream on checked exception
> This method may fail to clean up (close, dispose of) a stream, database
> object, or other resource requiring an explicit cleanup operation.
> In general, if a method opens a stream or other resource, the method should
> use a try/finally block to ensure that the stream or resource is cleaned up
> before the method returns.
> This bug pattern is essentially the same as the OS_OPEN_STREAM and
> ODR_OPEN_DATABASE_RESOURCE bug patterns, but is based on a different (and
> hopefully better) static analysis technique. We are interested is getting
> feedback about the usefulness of this bug pattern. To send feedback, either:
> send email to [email protected]
> file a bug report: http://findbugs.sourceforge.net/reportingBugs.html
> In particular, the false-positive suppression heuristics for this bug pattern
> have not been extensively tuned, so reports about false positives are helpful
> to us.
> See Weimer and Necula, Finding and Preventing Run-Time Error Handling
> Mistakes, for a description of the analysis technique.
> DataServices.java:663, OS_OPEN_STREAM_EXCEPTION_PATH
> - OS:
> org.apache.ofbiz.content.data.DataServices.updateBinaryFileMethod(DispatchContext,
> Map) may fail to close stream on exception
> The method creates an IO stream object, does not assign it to any fields,
> pass it to other methods, or return it, and does not appear to close it on
> all possible exception paths out of the method. This may result in a file
> descriptor leak. It is generally a good idea to use a finally block to
> ensure that streams are closed.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)