Mailing-List: contact notifications-help@ofbiz.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@ofbiz.apache.org
Date: Sun, 10 Dec 2017 10:32:00 +0000 (UTC)
From: "Michael Brohl (JIRA)" <jira@apache.org>
To: notifications@ofbiz.apache.org
Message-ID: <JIRA.13107434.1507275493000.425948.1512901920126@Atlassian.JIRA>
In-Reply-To: <JIRA.13107434.1507275493000@Atlassian.JIRA>
References: <JIRA.13107434.1507275493000@Atlassian.JIRA> <JIRA.13107434.1507275493453@jira-lw-us.apache.org>
Subject: [jira] [Closed] (OFBIZ-9816) [FB] Package
 org.apache.ofbiz.content.survey
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
archived-at: Sun, 10 Dec 2017 10:32:08 -0000


     [ 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)