ofbiz-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Michael Brohl (JIRA)" <j...@apache.org>
Subject [jira] [Closed] (OFBIZ-9816) [FB] Package org.apache.ofbiz.content.survey
Date Sun, 10 Dec 2017 10:32:00 GMT

     [ https://issues.apache.org/jira/browse/OFBIZ-9816?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]

Michael Brohl closed OFBIZ-9816.
--------------------------------
       Resolution: Implemented
    Fix Version/s: Upcoming Release

Thanks Julian,

your patch is in trunk r1817686. 

I've modified the patch and added some logging in PdfSurveyServices.java
instead of just printing the stacktrace. This class needs more work to
change the logging (see OFBIZ-10046).

> [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
>             Fix For: Upcoming Release
>
>         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 findbugs@cs.umd.edu
>     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)

Mime
View raw message