From b2a33515e9662e015b0e0d6b4aa0726245db3231 Mon Sep 17 00:00:00 2001 From: Andreas Beeker Date: Sat, 7 Dec 2019 11:45:44 +0000 Subject: [PATCH] Sonar Fixes - type: bugs / severity: critical - mostly div by 0 git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1870976 13f79535-47bb-0310-9956-ffa450edef68 --- .../examples/AddDimensionedImage.java | 14 ++-- .../poi/ss/examples/AddDimensionedImage.java | 8 +- .../xssf/usermodel/examples/BigGridDemo.java | 3 +- .../poi/ss/excelant/ExcelAntEvaluateCell.java | 60 +++++++-------- .../org/apache/poi/sl/draw/geom/Guide.java | 46 +++++------- .../ss/formula/atp/YearFracCalculator.java | 28 +++---- .../apache/poi/ss/formula/functions/Irr.java | 15 +++- .../functions/LinearRegressionFunction.java | 73 +++++++------------ src/java/org/apache/poi/util/XMLHelper.java | 1 + 9 files changed, 113 insertions(+), 135 deletions(-) diff --git a/src/examples/src/org/apache/poi/hssf/usermodel/examples/AddDimensionedImage.java b/src/examples/src/org/apache/poi/hssf/usermodel/examples/AddDimensionedImage.java index 7a3fa8b3e6..ca027992a7 100644 --- a/src/examples/src/org/apache/poi/hssf/usermodel/examples/AddDimensionedImage.java +++ b/src/examples/src/org/apache/poi/hssf/usermodel/examples/AddDimensionedImage.java @@ -559,8 +559,8 @@ public class AddDimensionedImage { // Next, from the columns width, calculate how many co-ordinate // positons there are per millimetre - coordinatePositionsPerMM = ConvertImageUnits.TOTAL_COLUMN_COORDINATE_POSITIONS / - colWidthMM; + coordinatePositionsPerMM = colWidthMM == 0 ? 0 + : ConvertImageUnits.TOTAL_COLUMN_COORDINATE_POSITIONS / colWidthMM; // From this figure, determine how many co-ordinat positions to // inset the left hand or bottom edge of the image. inset = (int)(coordinatePositionsPerMM * overlapMM); @@ -615,8 +615,7 @@ public class AddDimensionedImage { row = sheet.createRow(toRow); } // Get the row's height in millimetres and add to the running total. - rowHeightMM = row.getHeightInPoints() / - ConvertImageUnits.POINTS_PER_MILLIMETRE; + rowHeightMM = row.getHeightInPoints() / ConvertImageUnits.POINTS_PER_MILLIMETRE; totalRowHeightMM += rowHeightMM; toRow++; } @@ -647,11 +646,10 @@ public class AddDimensionedImage { overlapMM = 0.0D; } - rowCoordinatesPerMM = ConvertImageUnits.TOTAL_ROW_COORDINATE_POSITIONS / - rowHeightMM; + rowCoordinatesPerMM = rowHeightMM == 0 ? 0 + : ConvertImageUnits.TOTAL_ROW_COORDINATE_POSITIONS / rowHeightMM; inset = (int)(overlapMM * rowCoordinatesPerMM); - clientAnchorDetail = new ClientAnchorDetail(startingRow, - toRow, inset); + clientAnchorDetail = new ClientAnchorDetail(startingRow, toRow, inset); } return(clientAnchorDetail); } diff --git a/src/examples/src/org/apache/poi/ss/examples/AddDimensionedImage.java b/src/examples/src/org/apache/poi/ss/examples/AddDimensionedImage.java index 82a2db46ee..89a77aba8b 100644 --- a/src/examples/src/org/apache/poi/ss/examples/AddDimensionedImage.java +++ b/src/examples/src/org/apache/poi/ss/examples/AddDimensionedImage.java @@ -460,8 +460,8 @@ public class AddDimensionedImage { // is then instantiated. if(sheet instanceof HSSFSheet) { colWidthMM = reqImageWidthMM; - colCoordinatesPerMM = ConvertImageUnits.TOTAL_COLUMN_COORDINATE_POSITIONS / - colWidthMM; + colCoordinatesPerMM = colWidthMM == 0 ? 0 + : ConvertImageUnits.TOTAL_COLUMN_COORDINATE_POSITIONS / colWidthMM; pictureWidthCoordinates = (int)(reqImageWidthMM * colCoordinatesPerMM); } @@ -553,8 +553,8 @@ public class AddDimensionedImage { ConvertImageUnits.POINTS_PER_MILLIMETRE)); if(sheet instanceof HSSFSheet) { rowHeightMM = reqImageHeightMM; - rowCoordinatesPerMM = ConvertImageUnits.TOTAL_ROW_COORDINATE_POSITIONS / - rowHeightMM; + rowCoordinatesPerMM = rowHeightMM == 0 ? 0 + : ConvertImageUnits.TOTAL_ROW_COORDINATE_POSITIONS / rowHeightMM; pictureHeightCoordinates = (int)(reqImageHeightMM * rowCoordinatesPerMM); } diff --git a/src/examples/src/org/apache/poi/xssf/usermodel/examples/BigGridDemo.java b/src/examples/src/org/apache/poi/xssf/usermodel/examples/BigGridDemo.java index 65d0686a8c..dd7e370d1b 100644 --- a/src/examples/src/org/apache/poi/xssf/usermodel/examples/BigGridDemo.java +++ b/src/examples/src/org/apache/poi/xssf/usermodel/examples/BigGridDemo.java @@ -78,6 +78,8 @@ import org.apache.poi.xssf.usermodel.XSSFWorkbook; public final class BigGridDemo { private static final String XML_ENCODING = "UTF-8"; + private static final Random rnd = new Random(); + private BigGridDemo() {} public static void main(String[] args) throws Exception { @@ -153,7 +155,6 @@ public final class BigGridDemo { private static void generate(Writer out, Map styles) throws Exception { - Random rnd = new Random(); Calendar calendar = Calendar.getInstance(); SpreadsheetWriter sw = new SpreadsheetWriter(out); diff --git a/src/excelant/java/org/apache/poi/ss/excelant/ExcelAntEvaluateCell.java b/src/excelant/java/org/apache/poi/ss/excelant/ExcelAntEvaluateCell.java index a624b98b57..eb446dfde6 100644 --- a/src/excelant/java/org/apache/poi/ss/excelant/ExcelAntEvaluateCell.java +++ b/src/excelant/java/org/apache/poi/ss/excelant/ExcelAntEvaluateCell.java @@ -27,9 +27,9 @@ import org.apache.tools.ant.Task; * Instances of this class are used to evaluate a single cell. This is usually * after some values have been set. The evaluation is actually performed * by a WorkbookUtil instance. The evaluate() method of the WorkbookUtil - * class returns an EvaluationResult which encapsulates the results and + * class returns an EvaluationResult which encapsulates the results and * information from the evaluation. - * + * * @author Jon Svede ( jon [at] loquatic [dot] com ) * @author Brian Bush ( brian [dot] bush [at] nrel [dot] gov ) @@ -40,40 +40,39 @@ public class ExcelAntEvaluateCell extends Task { private String cell ; private double expectedValue ; private double precision ; - private double precisionToUse ; private double globalPrecision ; private boolean requiredToPass; - - + + private ExcelAntEvaluationResult result ; - + private ExcelAntWorkbookUtil wbUtil ; - + private boolean showDelta; - - + + public ExcelAntEvaluateCell() {} protected void setWorkbookUtil( ExcelAntWorkbookUtil wb ) { wbUtil = wb ; } - + public void setShowDelta( boolean value ) { showDelta = value ; } - + protected boolean showDelta() { return showDelta ; } - + public void setCell(String cell) { this.cell = cell; } - + public void setRequiredToPass( boolean val ) { requiredToPass = val ; } - + protected boolean requiredToPass() { return requiredToPass ; } @@ -85,7 +84,7 @@ public class ExcelAntEvaluateCell extends Task { public void setPrecision(double precision) { this.precision = precision; } - + protected void setGlobalPrecision( double prec ) { globalPrecision = prec ; } @@ -98,47 +97,44 @@ public class ExcelAntEvaluateCell extends Task { return expectedValue; } + @SuppressWarnings("squid:S4275") protected double getPrecision() { - return precisionToUse; - } - - @Override - public void execute() throws BuildException { - - precisionToUse = 0 ; - // if there is a globalPrecision we will use it unless there is also // precision set at the evaluate level, then we use that. If there // is not a globalPrecision, we will use the local precision. log( "test precision = " + precision + "\tglobal precision = " + globalPrecision, Project.MSG_VERBOSE ) ; if( globalPrecision > 0 ) { if( precision > 0 ) { - precisionToUse = precision ; log( "Using evaluate precision of " + precision + " over the " + - "global precision of " + globalPrecision, Project.MSG_VERBOSE ) ; + "global precision of " + globalPrecision, Project.MSG_VERBOSE ) ; + return precision ; } else { - precisionToUse = globalPrecision ; log( "Using global precision of " + globalPrecision, Project.MSG_VERBOSE ) ; + return globalPrecision ; } } else { - precisionToUse = precision ; log( "Using evaluate precision of " + precision, Project.MSG_VERBOSE ) ; + return precision ; } - result = wbUtil.evaluateCell(cell, expectedValue, precisionToUse ) ; - + } + + @Override + public void execute() throws BuildException { + result = wbUtil.evaluateCell(cell, expectedValue, getPrecision() ) ; + StringBuilder sb = new StringBuilder() ; sb.append( "evaluation of cell " ) ; - sb.append( cell ) ; + sb.append( cell ) ; sb.append( " resulted in " ) ; sb.append( result.getReturnValue() ) ; if(showDelta) { sb.append(" with a delta of ").append(result.getDelta()); } - + log( sb.toString(), Project.MSG_DEBUG) ; } - + public ExcelAntEvaluationResult getResult() { return result ; } diff --git a/src/java/org/apache/poi/sl/draw/geom/Guide.java b/src/java/org/apache/poi/sl/draw/geom/Guide.java index 7b60d9e551..323bca07d0 100644 --- a/src/java/org/apache/poi/sl/draw/geom/Guide.java +++ b/src/java/org/apache/poi/sl/draw/geom/Guide.java @@ -19,16 +19,9 @@ package org.apache.poi.sl.draw.geom; -import static java.lang.Math.abs; -import static java.lang.Math.atan2; -import static java.lang.Math.cos; -import static java.lang.Math.max; -import static java.lang.Math.min; -import static java.lang.Math.sin; -import static java.lang.Math.sqrt; -import static java.lang.Math.tan; -import static java.lang.Math.toDegrees; -import static java.lang.Math.toRadians; +import static java.lang.Math.*; + +import java.util.regex.Pattern; import org.apache.poi.sl.draw.binding.CTGeomGuide; @@ -40,6 +33,9 @@ public class Guide implements Formula { muldiv,addsub,adddiv,ifelse,val,abs,sqrt,max,min,at2,sin,cos,tan,cat2,sat2,pin,mod } + private static final Pattern WHITESPACE = Pattern.compile("\\s+"); + + @SuppressWarnings({"FieldCanBeLocal", "unused"}) private final String name, fmla; private final Op op; private final String[] operands; @@ -52,18 +48,20 @@ public class Guide implements Formula { public Guide(String nm, String fm){ name = nm; fmla = fm; - operands = fm.split("\\s+"); - op = Op.valueOf(operands[0].replace("*", "mul").replace("/", "div").replace("+", "add").replace("-", "sub").replace("?:", "ifelse")); + operands = WHITESPACE.split(fm); + switch (operands[0]) { + case "*/": op = Op.muldiv; break; + case "+-": op = Op.addsub; break; + case "+/": op = Op.adddiv; break; + case "?:": op = Op.ifelse; break; + default: op = Op.valueOf(operands[0]); break; + } } public String getName(){ return name; } - String getFormula(){ - return fmla; - } - @Override public double evaluate(Context ctx) { double x = (operands.length > 1) ? ctx.getValue(operands[1]) : 0; @@ -75,7 +73,7 @@ public class Guide implements Formula { return abs(x); case adddiv: // Add Divide Formula - return (x + y) / z; + return (z == 0) ? 0 : (x + y) / z; case addsub: // Add Subtract Formula return (x + y) - z; @@ -87,7 +85,7 @@ public class Guide implements Formula { return x * cos(toRadians(y / OOXML_DEGREE)); case cat2: // Cosine ArcTan Formula: "cat2 x y z" = (x * cos(arctan(z / y) )) = value of this guide - return x*cos(atan2(z, y)); + return x * cos(atan2(z, y)); case ifelse: // If Else Formula: "?: x y z" = if (x > 0), then y = value of this guide, // else z = value of this guide @@ -106,21 +104,15 @@ public class Guide implements Formula { return sqrt(x*x + y*y + z*z); case muldiv: // Multiply Divide Formula - return (x * y) / z; + return (z == 0) ? 0 : (x * y) / z; case pin: // Pin To Formula: "pin x y z" = if (y < x), then x = value of this guide // else if (y > z), then z = value of this guide // else y = value of this guide - if(y < x) { - return x; - } else if (y > z) { - return z; - } else { - return y; - } + return max(x, min(y, z)); case sat2: // Sine ArcTan Formula: "sat2 x y z" = (x*sin(arctan(z / y))) = value of this guide - return x*sin(atan2(z, y)); + return x * sin(atan2(z, y)); case sin: // Sine Formula: "sin x y" = (x * sin( y )) = value of this guide return x * sin(toRadians(y / OOXML_DEGREE)); diff --git a/src/java/org/apache/poi/ss/formula/atp/YearFracCalculator.java b/src/java/org/apache/poi/ss/formula/atp/YearFracCalculator.java index 7a31656aed..099df44c8c 100644 --- a/src/java/org/apache/poi/ss/formula/atp/YearFracCalculator.java +++ b/src/java/org/apache/poi/ss/formula/atp/YearFracCalculator.java @@ -22,6 +22,7 @@ import java.util.Calendar; import org.apache.poi.ss.formula.eval.ErrorEval; import org.apache.poi.ss.formula.eval.EvaluationException; import org.apache.poi.ss.usermodel.DateUtil; +import org.apache.poi.util.Internal; import org.apache.poi.util.LocaleUtil; @@ -30,6 +31,7 @@ import org.apache.poi.util.LocaleUtil; * * Algorithm inspired by www.dwheeler.com/yearfrac */ +@Internal final class YearFracCalculator { private static final int MS_PER_HOUR = 60 * 60 * 1000; private static final int MS_PER_DAY = 24 * MS_PER_HOUR; @@ -86,7 +88,7 @@ final class YearFracCalculator { * @param startDateVal assumed to be less than or equal to endDateVal * @param endDateVal assumed to be greater than or equal to startDateVal */ - public static double basis0(int startDateVal, int endDateVal) { + private static double basis0(int startDateVal, int endDateVal) { SimpleDate startDate = createDate(startDateVal); SimpleDate endDate = createDate(endDateVal); int date1day = startDate.day; @@ -116,12 +118,14 @@ final class YearFracCalculator { * @param startDateVal assumed to be less than or equal to endDateVal * @param endDateVal assumed to be greater than or equal to startDateVal */ - public static double basis1(int startDateVal, int endDateVal) { + private static double basis1(int startDateVal, int endDateVal) { + assert(startDateVal <= endDateVal); SimpleDate startDate = createDate(startDateVal); SimpleDate endDate = createDate(endDateVal); double yearLength; if (isGreaterThanOneYear(startDate, endDate)) { yearLength = averageYearLength(startDate.year, endDate.year); + assert(yearLength > 0); } else if (shouldCountFeb29(startDate, endDate)) { yearLength = DAYS_PER_LEAP_YEAR; } else { @@ -134,21 +138,21 @@ final class YearFracCalculator { * @param startDateVal assumed to be less than or equal to endDateVal * @param endDateVal assumed to be greater than or equal to startDateVal */ - public static double basis2(int startDateVal, int endDateVal) { + private static double basis2(int startDateVal, int endDateVal) { return (endDateVal - startDateVal) / 360.0; } /** * @param startDateVal assumed to be less than or equal to endDateVal * @param endDateVal assumed to be greater than or equal to startDateVal */ - public static double basis3(double startDateVal, double endDateVal) { + private static double basis3(double startDateVal, double endDateVal) { return (endDateVal - startDateVal) / 365.0; } /** * @param startDateVal assumed to be less than or equal to endDateVal * @param endDateVal assumed to be greater than or equal to startDateVal */ - public static double basis4(int startDateVal, int endDateVal) { + private static double basis4(int startDateVal, int endDateVal) { SimpleDate startDate = createDate(startDateVal); SimpleDate endDate = createDate(endDateVal); int date1day = startDate.day; @@ -261,12 +265,10 @@ final class YearFracCalculator { } private static double averageYearLength(int startYear, int endYear) { + assert(startYear <= endYear); int dayCount = 0; for (int i=startYear; i<=endYear; i++) { - dayCount += DAYS_PER_NORMAL_YEAR; - if (isLeapYear(i)) { - dayCount++; - } + dayCount += isLeapYear(i) ? DAYS_PER_LEAP_YEAR : DAYS_PER_NORMAL_YEAR; } double numberOfYears = endYear-startYear+1; return dayCount / numberOfYears; @@ -282,13 +284,11 @@ final class YearFracCalculator { return true; } // all other centuries are *not* leap years - if (i % 100 == 0) { - return false; - } - return true; + return i % 100 != 0; } private static boolean isGreaterThanOneYear(SimpleDate start, SimpleDate end) { + assert(start.year <= end.year); if (start.year == end.year) { return false; } @@ -307,7 +307,7 @@ final class YearFracCalculator { } private static SimpleDate createDate(int dayCount) { - /** use UTC time-zone to avoid daylight savings issues */ + /* use UTC time-zone to avoid daylight savings issues */ Calendar cal = LocaleUtil.getLocaleCalendar(LocaleUtil.TIMEZONE_UTC); DateUtil.setCalendar(cal, dayCount, 0, false, false); return new SimpleDate(cal); diff --git a/src/java/org/apache/poi/ss/formula/functions/Irr.java b/src/java/org/apache/poi/ss/formula/functions/Irr.java index 1384793ffc..5d91be9a74 100644 --- a/src/java/org/apache/poi/ss/formula/functions/Irr.java +++ b/src/java/org/apache/poi/ss/formula/functions/Irr.java @@ -17,7 +17,10 @@ package org.apache.poi.ss.formula.functions; -import org.apache.poi.ss.formula.eval.*; +import org.apache.poi.ss.formula.eval.ErrorEval; +import org.apache.poi.ss.formula.eval.EvaluationException; +import org.apache.poi.ss.formula.eval.NumberEval; +import org.apache.poi.ss.formula.eval.ValueEval; /** * Calculates the internal rate of return. @@ -104,11 +107,17 @@ public final class Irr implements Function { final double value = values[k]; fValue += value / denominator; denominator *= factor; + if (denominator == 0) { + return Double.NaN; + } fDerivative -= k * value / denominator; } - // the essense of the Newton-Raphson Method - x1 = x0 - fValue/fDerivative; + // the essence of the Newton-Raphson Method + if (fDerivative == 0) { + return Double.NaN; + } + x1 = x0 - fValue/fDerivative; if (Math.abs(x1 - x0) <= absoluteAccuracy) { return x1; diff --git a/src/java/org/apache/poi/ss/formula/functions/LinearRegressionFunction.java b/src/java/org/apache/poi/ss/formula/functions/LinearRegressionFunction.java index e36094c26f..aee79ab274 100644 --- a/src/java/org/apache/poi/ss/formula/functions/LinearRegressionFunction.java +++ b/src/java/org/apache/poi/ss/formula/functions/LinearRegressionFunction.java @@ -41,7 +41,7 @@ import org.apache.poi.ss.formula.functions.LookupUtils.ValueVector; * @author Johan Karlsteen */ public final class LinearRegressionFunction extends Fixed2ArgFunction { - + private static abstract class ValueArray implements ValueVector { private final int _size; protected ValueArray(int size) { @@ -84,7 +84,7 @@ public final class LinearRegressionFunction extends Fixed2ArgFunction { } protected ValueEval getItemInternal(int index) { - int sIx = (index % _width) + _ref.getFirstSheetIndex(); + int sIx = (index % _width) + _ref.getFirstSheetIndex(); return _ref.getInnerValueEval(sIx); } } @@ -108,11 +108,11 @@ public final class LinearRegressionFunction extends Fixed2ArgFunction { public enum FUNCTION {INTERCEPT, SLOPE} public FUNCTION function; - + public LinearRegressionFunction(FUNCTION function) { this.function = function; } - + public ValueEval evaluate(int srcRowIndex, int srcColumnIndex, ValueEval arg0, ValueEval arg1) { double result; @@ -132,25 +132,21 @@ public final class LinearRegressionFunction extends Fixed2ArgFunction { } return new NumberEval(result); } - + private double evaluateInternal(ValueVector x, ValueVector y, int size) throws EvaluationException { // error handling is as if the x is fully evaluated before y - ErrorEval firstXerr = null; ErrorEval firstYerr = null; boolean accumlatedSome = false; // first pass: read in data, compute xbar and ybar double sumx = 0.0, sumy = 0.0; - + for (int i = 0; i < size; i++) { ValueEval vx = x.getItem(i); ValueEval vy = y.getItem(i); if (vx instanceof ErrorEval) { - if (firstXerr == null) { - firstXerr = (ErrorEval) vx; - continue; - } + throw new EvaluationException((ErrorEval) vx); } if (vy instanceof ErrorEval) { if (firstYerr == null) { @@ -159,66 +155,51 @@ public final class LinearRegressionFunction extends Fixed2ArgFunction { } } // only count pairs if both elements are numbers + // all other combinations of value types are silently ignored if (vx instanceof NumberEval && vy instanceof NumberEval) { accumlatedSome = true; NumberEval nx = (NumberEval) vx; NumberEval ny = (NumberEval) vy; sumx += nx.getNumberValue(); sumy += ny.getNumberValue(); - } else { - // all other combinations of value types are silently ignored } } + + if (firstYerr != null) { + throw new EvaluationException(firstYerr); + } + + if (!accumlatedSome) { + throw new EvaluationException(ErrorEval.DIV_ZERO); + } + double xbar = sumx / size; double ybar = sumy / size; - + // second pass: compute summary statistics double xxbar = 0.0, xybar = 0.0; for (int i = 0; i < size; i++) { ValueEval vx = x.getItem(i); ValueEval vy = y.getItem(i); - - if (vx instanceof ErrorEval) { - if (firstXerr == null) { - firstXerr = (ErrorEval) vx; - continue; - } - } - if (vy instanceof ErrorEval) { - if (firstYerr == null) { - firstYerr = (ErrorEval) vy; - continue; - } - } - + // only count pairs if both elements are numbers + // all other combinations of value types are silently ignored if (vx instanceof NumberEval && vy instanceof NumberEval) { NumberEval nx = (NumberEval) vx; NumberEval ny = (NumberEval) vy; xxbar += (nx.getNumberValue() - xbar) * (nx.getNumberValue() - xbar); xybar += (nx.getNumberValue() - xbar) * (ny.getNumberValue() - ybar); - } else { - // all other combinations of value types are silently ignored } } - double beta1 = xybar / xxbar; - double beta0 = ybar - beta1 * xbar; - - if (firstXerr != null) { - throw new EvaluationException(firstXerr); - } - if (firstYerr != null) { - throw new EvaluationException(firstYerr); - } - if (!accumlatedSome) { + + if (xxbar == 0 ) { throw new EvaluationException(ErrorEval.DIV_ZERO); } - - if(function == FUNCTION.INTERCEPT) { - return beta0; - } else { - return beta1; - } + + double beta1 = xybar / xxbar; + double beta0 = ybar - beta1 * xbar; + + return (function == FUNCTION.INTERCEPT) ? beta0 : beta1; } private static ValueVector createValueVector(ValueEval arg) throws EvaluationException { diff --git a/src/java/org/apache/poi/util/XMLHelper.java b/src/java/org/apache/poi/util/XMLHelper.java index ab408c4444..c1af6b2ab0 100644 --- a/src/java/org/apache/poi/util/XMLHelper.java +++ b/src/java/org/apache/poi/util/XMLHelper.java @@ -213,6 +213,7 @@ public final class XMLHelper { return XMLEventFactory.newInstance(); } + @SuppressWarnings("squid:S4435") public static TransformerFactory getTransformerFactory() { TransformerFactory factory = TransformerFactory.newInstance(); trySet(factory::setFeature, FEATURE_SECURE_PROCESSING, true); -- 2.39.5