[
https://issues.apache.org/jira/browse/OFBIZ-9816?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Michael Brohl reassigned OFBIZ-9816:
------------------------------------
Assignee: Michael Brohl
> [FB] Package org.apache.ofbiz.content.survey
> --------------------------------------------
>
> Key: OFBIZ-9816
> URL: https://issues.apache.org/jira/browse/OFBIZ-9816
> Project: OFBiz
> Issue Type: Sub-task
> Components: content
> Affects Versions: Trunk
> Reporter: Julian Leichert
> Assignee: Michael Brohl
> Priority: Minor
> Attachments: OFBIZ-9816_org.apache.ofbiz.content.survey_bugfixes.patch
>
>
> PdfSurveyServices.java:214, REC_CATCH_EXCEPTION
> - REC: Exception is caught when Exception is not thrown in
> org.apache.ofbiz.content.survey.PdfSurveyServices.buildSurveyFromPdf(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 ...
> }
> PdfSurveyServices.java:283, REC_CATCH_EXCEPTION
> - REC: Exception is caught when Exception is not thrown in
> org.apache.ofbiz.content.survey.PdfSurveyServices.buildSurveyResponseFromPdf(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 ...
> }
> PdfSurveyServices.java:557, OS_OPEN_STREAM_EXCEPTION_PATH
> - OS:
> org.apache.ofbiz.content.survey.PdfSurveyServices.setAcroFieldsFromSurveyResponse(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.
> PdfSurveyServices.java:557, OBL_UNSATISFIED_OBLIGATION_EXCEPTION_EDGE
> - OBL:
> org.apache.ofbiz.content.survey.PdfSurveyServices.setAcroFieldsFromSurveyResponse(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.
> SurveyWrapper.java:227, DM_DEFAULT_ENCODING
> - Dm: Found reliance on default encoding in
> org.apache.ofbiz.content.survey.SurveyWrapper.getTemplate(URL): new
> java.io.InputStreamReader(InputStream)
> 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.
> SurveyWrapper.java:378, DM_CONVERT_CASE
> - Dm: Use of non-localized String.toUpperCase() or String.toLowerCase() in
> org.apache.ofbiz.content.survey.SurveyWrapper.getResponseAnswers(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.
> SurveyWrapper.java:469, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE
> - RCN: Redundant nullcheck of thisResult, which is known to be non-null in
> org.apache.ofbiz.content.survey.SurveyWrapper.getResultInfo(GenericValue)
> This method contains a redundant check of a known non-null value against the
> constant null.
> SurveyWrapper.java:480, WMI_WRONG_MAP_ITERATOR
> - WMI:
> org.apache.ofbiz.content.survey.SurveyWrapper.getResultInfo(GenericValue)
> makes inefficient use of keySet iterator instead of entrySet iterator
> This method accesses the value of a Map entry, using a key that was retrieved
> from a keySet iterator. It is more efficient to use an iterator on the
> entrySet of the map, to avoid the Map.get(key) lookup.
> SurveyWrapper.java:592, SF_SWITCH_NO_DEFAULT
> - SF: Switch statement found in
> org.apache.ofbiz.content.survey.SurveyWrapper.getNumberResult(GenericValue,
> int) where default case is missing
> This method contains a switch statement where default case is missing.
> Usually you need to provide a default case.
> Because the analysis only looks at the generated bytecode, this warning can
> be incorrect triggered if the default case is at the end of the switch
> statement and the switch statement doesn't contain break statements for other
> cases.
> SurveyWrapper.java:637, ICAST_IDIV_CAST_TO_DOUBLE
> - ICAST: Integral division result cast to double or float in
> org.apache.ofbiz.content.survey.SurveyWrapper.getNumberResult(GenericValue,
> 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;
> SurveyWrapper.java:734, SIC_INNER_SHOULD_BE_STATIC
> - SIC: Should
> org.apache.ofbiz.content.survey.SurveyWrapper$SurveyWrapperException be a
> _static_ inner class?
> This class is an inner class, but does not use its embedded reference to the
> object which created it. This reference makes the instances of the class
> larger, and may keep the reference to the creator object alive longer than
> necessary. If possible, the class should be made static.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
