From 998b78009bba35280da741a599f3edd8b62c93d0 Mon Sep 17 00:00:00 2001 From: Armin Samii Date: Mon, 22 Jan 2024 20:47:19 -0500 Subject: [PATCH] check if ranking is allowed rather than using unacceptable default --- .../brightspots/rcv/BaseCvrReader.java | 4 +- .../brightspots/rcv/ClearBallotCvrReader.java | 6 +-- .../brightspots/rcv/ContestConfig.java | 41 +++++++++++++++---- .../brightspots/rcv/DominionCvrReader.java | 4 +- .../brightspots/rcv/ResultsWriter.java | 16 ++++++-- .../brightspots/rcv/StreamingCvrReader.java | 6 ++- .../network/brightspots/rcv/Tabulator.java | 4 +- .../brightspots/rcv/TabulatorSession.java | 3 +- 8 files changed, 62 insertions(+), 22 deletions(-) diff --git a/src/main/java/network/brightspots/rcv/BaseCvrReader.java b/src/main/java/network/brightspots/rcv/BaseCvrReader.java index 96cdd23d..80b5b36b 100644 --- a/src/main/java/network/brightspots/rcv/BaseCvrReader.java +++ b/src/main/java/network/brightspots/rcv/BaseCvrReader.java @@ -40,8 +40,8 @@ abstract void readCastVoteRecords(List castVoteRecords) throws CastVoteRecord.CvrParseException, IOException; // Individual contests may have a different value than what the config allows. - public Integer getMaxRankingsAllowed(String contestId) { - return config.getMaxRankingsAllowed(); + public boolean isRankingAllowed(int rank, String contestId) { + return config.isRankingAllowed(rank); } // Any reader-specific validations can override this function. diff --git a/src/main/java/network/brightspots/rcv/ClearBallotCvrReader.java b/src/main/java/network/brightspots/rcv/ClearBallotCvrReader.java index 78a5552b..470dc3cb 100644 --- a/src/main/java/network/brightspots/rcv/ClearBallotCvrReader.java +++ b/src/main/java/network/brightspots/rcv/ClearBallotCvrReader.java @@ -82,10 +82,10 @@ void readCastVoteRecords(List castVoteRecords) choiceName = Tabulator.UNDECLARED_WRITE_IN_OUTPUT_LABEL; } Integer rank = Integer.parseInt(choiceFields[RcvChoiceHeaderField.RANK.ordinal()]); - if (rank > this.config.getMaxRankingsAllowed()) { + if (!this.config.isRankingAllowed(rank)) { Logger.severe( - "Rank: %d exceeds max rankings allowed in config: %d", - rank, this.config.getMaxRankingsAllowed()); + "Rank: %d exceeds max rankings allowed in config: %s", + rank, this.config.getMaxRankingsAllowedAsString()); throw new CvrParseException(); } columnIndexToRanking.put(columnIndex, new Pair<>(rank, choiceName)); diff --git a/src/main/java/network/brightspots/rcv/ContestConfig.java b/src/main/java/network/brightspots/rcv/ContestConfig.java index ce04046f..4b31aa92 100644 --- a/src/main/java/network/brightspots/rcv/ContestConfig.java +++ b/src/main/java/network/brightspots/rcv/ContestConfig.java @@ -678,9 +678,9 @@ private void validateRules() { Logger.severe("Invalid winnerElectionMode!"); } - if (getMaxRankingsAllowed() == null + if (getMaxRankingsAllowedAsString() == null || (getNumDeclaredCandidates() >= 1 - && getMaxRankingsAllowed() < MIN_MAX_RANKINGS_ALLOWED)) { + && !isRankingAllowed(MIN_MAX_RANKINGS_ALLOWED))) { validationErrors.add(ValidationError.RULES_MAX_RANKINGS_ALLOWED_INVALID); Logger.severe( "maxRankingsAllowed must either be \"%s\" or an integer from %d to %d!", @@ -988,11 +988,38 @@ private String getMaxRankingsAllowedRaw() { return rawConfig.rules.maxRankingsAllowed; } - Integer getMaxRankingsAllowed() { - return stringToIntWithOption( - getMaxRankingsAllowedRaw(), - MAX_RANKINGS_ALLOWED_NUM_CANDIDATES_OPTION, - getNumDeclaredCandidates()); + // Because the max rank can be set to "max", there's no reasonable value we can + // return outward. Returning MAX_VALUE could cause integer overflows if + // the caller tries to do arithmetic with the result (e.g. convert to a max column) + // Instead of returning the max rank, only allow checking via this function, + // or returning the value as a string for audit logging. + boolean isRankingAllowed(int rank) { + if (isMaxRankingsSetToMaximum()) { + return true; + } + + return rank <= Integer.parseInt(getMaxRankingsAllowedRaw()); + } + + // There are times when it is necessary to grab the max ranking, though: + // notably, when writing outputs up until the max ranking, or when + // reading inputs where a "blank" indicates an undeclared write-in. + // Handle that, but ensure the caller has checked if it's set to "max" + Integer getMaxRankingsAllowedWhenNotSetToMaximum() { + if (isMaxRankingsSetToMaximum()) { + throw new RuntimeException( + "Do not call this function without first checking isMaxRankingsSetToMaximum!"); + } + + return Integer.parseInt(getMaxRankingsAllowedRaw()); + } + + boolean isMaxRankingsSetToMaximum() { + return getMaxRankingsAllowedRaw().equals(MAX_RANKINGS_ALLOWED_NUM_CANDIDATES_OPTION); + } + + String getMaxRankingsAllowedAsString() { + return getMaxRankingsAllowedRaw(); } boolean isBatchEliminationEnabled() { diff --git a/src/main/java/network/brightspots/rcv/DominionCvrReader.java b/src/main/java/network/brightspots/rcv/DominionCvrReader.java index 19821b0e..1d884e1f 100644 --- a/src/main/java/network/brightspots/rcv/DominionCvrReader.java +++ b/src/main/java/network/brightspots/rcv/DominionCvrReader.java @@ -175,8 +175,8 @@ public void runAdditionalValidations(List castVoteRecords) } @Override - public Integer getMaxRankingsAllowed(String contestId) { - return contests.get(contestId).getMaxRanks(); + public boolean isRankingAllowed(int rank, String contestId) { + return rank <= contests.get(contestId).getMaxRanks(); } private void validateNamesAreInContest(List castVoteRecords) diff --git a/src/main/java/network/brightspots/rcv/ResultsWriter.java b/src/main/java/network/brightspots/rcv/ResultsWriter.java index 17489098..a8de8362 100644 --- a/src/main/java/network/brightspots/rcv/ResultsWriter.java +++ b/src/main/java/network/brightspots/rcv/ResultsWriter.java @@ -542,7 +542,8 @@ void generateOverallSummaryFiles( // returns the filepath written String writeGenericCvrCsv( List castVoteRecords, - Integer numRanks, + CvrSource source, + BaseCvrReader reader, String csvOutputFolder, String inputFilepath, String contestId, @@ -581,7 +582,11 @@ String writeGenericCvrCsv( csvPrinter.print("Record Id"); csvPrinter.print("Precinct"); csvPrinter.print("Precinct Portion"); - for (int rank = 1; rank <= numRanks; rank++) { + int maxRank = 0; + for (CastVoteRecord castVoteRecord : castVoteRecords) { + maxRank = Math.max(maxRank, castVoteRecord.candidateRankings.maxRankingNumber()); + } + for (int rank = 1; rank <= maxRank; rank++) { String label = String.format("Rank %d", rank); csvPrinter.print(label); } @@ -602,7 +607,7 @@ String writeGenericCvrCsv( } else { csvPrinter.print(castVoteRecord.getPrecinctPortion()); } - printRankings(undeclaredWriteInLabel, numRanks, csvPrinter, castVoteRecord); + printRankings(undeclaredWriteInLabel, maxRank, reader, source, csvPrinter, castVoteRecord); csvPrinter.println(); } // finalize the file @@ -624,12 +629,17 @@ String writeGenericCvrCsv( private void printRankings( String undeclaredWriteInLabel, Integer maxRanks, + BaseCvrReader reader, + CvrSource source, CSVPrinter csvPrinter, CastVoteRecord castVoteRecord) throws IOException { // for each rank determine what candidate id, overvote, or undervote occurred and print it for (int rank = 1; rank <= maxRanks; rank++) { if (castVoteRecord.candidateRankings.hasRankingAt(rank)) { + if (reader.isRankingAllowed(rank, source.getContestId())) { + break; + } CandidatesAtRanking candidates = castVoteRecord.candidateRankings.get(rank); if (candidates.count() == 1) { String selection = candidates.get(0); diff --git a/src/main/java/network/brightspots/rcv/StreamingCvrReader.java b/src/main/java/network/brightspots/rcv/StreamingCvrReader.java index 90e2f1cb..6cd75fd0 100644 --- a/src/main/java/network/brightspots/rcv/StreamingCvrReader.java +++ b/src/main/java/network/brightspots/rcv/StreamingCvrReader.java @@ -166,7 +166,9 @@ private void beginCvr() { // complete construction of new CVR object private void endCvr() { // handle any empty cells which may appear at the end of this row - handleEmptyCells(config.getMaxRankingsAllowed() + 1); + if (!config.isMaxRankingsSetToMaximum()) { + handleEmptyCells(config.getMaxRankingsAllowedWhenNotSetToMaximum() + 1); + } String computedCastVoteRecordId = String.format("%s-%d", ResultsWriter.sanitizeStringForOutput(excelFileName), cvrIndex); @@ -215,7 +217,7 @@ private void cvrCell(int col, String cellData) { // see if this column is in the ranking range if (col >= firstVoteColumnIndex - && col < firstVoteColumnIndex + config.getMaxRankingsAllowed()) { + && config.isRankingAllowed(col - firstVoteColumnIndex)) { int currentRank = col - firstVoteColumnIndex + 1; // handle any empty cells which may exist between this cell and any previous one handleEmptyCells(currentRank); diff --git a/src/main/java/network/brightspots/rcv/Tabulator.java b/src/main/java/network/brightspots/rcv/Tabulator.java index 14bbf954..bae84349 100644 --- a/src/main/java/network/brightspots/rcv/Tabulator.java +++ b/src/main/java/network/brightspots/rcv/Tabulator.java @@ -1132,8 +1132,8 @@ && isCandidateContinuing(cvr.getCurrentRecipientOfVote())) { // if this is the last ranking we are out of rankings and must exhaust this cvr // determine if the reason is skipping too many ranks, or no continuing candidates if (rank == cvr.candidateRankings.maxRankingNumber()) { - if (config.getMaxSkippedRanksAllowed() != Integer.MAX_VALUE - && config.getMaxRankingsAllowed() - rank > config.getMaxSkippedRanksAllowed()) { + if (!config.isMaxRankingsSetToMaximum() + && config.getMaxRankingsAllowedWhenNotSetToMaximum() - rank > config.getMaxSkippedRanksAllowed()) { recordSelectionForCastVoteRecord( cvr, roundTally, null, StatusForRound.INACTIVE_BY_UNDERVOTE, ""); } else { diff --git a/src/main/java/network/brightspots/rcv/TabulatorSession.java b/src/main/java/network/brightspots/rcv/TabulatorSession.java index e41ab06a..427c72f2 100644 --- a/src/main/java/network/brightspots/rcv/TabulatorSession.java +++ b/src/main/java/network/brightspots/rcv/TabulatorSession.java @@ -306,7 +306,8 @@ private List parseCastVoteRecords(ContestConfig config) { this.convertedFilePath = writer.writeGenericCvrCsv( castVoteRecords, - reader.getMaxRankingsAllowed(source.getContestId()), + source, + reader, config.getOutputDirectory(), source.getFilePath(), source.getContestId(),