From 12dfc4d353661e619e87697338f9cdefadf506ae Mon Sep 17 00:00:00 2001 From: NebelNidas Date: Mon, 15 Apr 2024 12:00:10 +0200 Subject: [PATCH] Add tests --- .../mappingio/format/ColumnFileReader.java | 8 +- .../format/enigma/EnigmaFileReader.java | 20 +- .../mappingio/format/srg/SrgFileReader.java | 1 + .../mappingio/format/srg/TsrgFileReader.java | 8 +- .../format/tiny/Tiny2FileReader.java | 58 +++- .../read/InvalidContentReadTest.java | 294 ++++++++++++++++++ 6 files changed, 371 insertions(+), 18 deletions(-) create mode 100644 src/test/java/net/fabricmc/mappingio/read/InvalidContentReadTest.java diff --git a/src/main/java/net/fabricmc/mappingio/format/ColumnFileReader.java b/src/main/java/net/fabricmc/mappingio/format/ColumnFileReader.java index 4d26aa0b..06a0f029 100644 --- a/src/main/java/net/fabricmc/mappingio/format/ColumnFileReader.java +++ b/src/main/java/net/fabricmc/mappingio/format/ColumnFileReader.java @@ -227,13 +227,17 @@ public String peekCols(boolean unescape) throws IOException { * * @return -1 if nothing has been read (first char was EOL), otherwise the number present. */ - public int nextIntCol() throws IOException { + public int nextIntCol(boolean rethrowNumberFormatExceptionAsIOException) throws IOException { String str = nextCol(false); try { return str != null ? Integer.parseInt(str) : -1; } catch (NumberFormatException e) { - throw new IOException("invalid number in line "+lineNumber+": "+str); + if (rethrowNumberFormatExceptionAsIOException) { + throw new IOException("invalid number in line "+lineNumber+": "+str); + } else { + throw e; + } } } diff --git a/src/main/java/net/fabricmc/mappingio/format/enigma/EnigmaFileReader.java b/src/main/java/net/fabricmc/mappingio/format/enigma/EnigmaFileReader.java index dee974fb..b691f0a7 100644 --- a/src/main/java/net/fabricmc/mappingio/format/enigma/EnigmaFileReader.java +++ b/src/main/java/net/fabricmc/mappingio/format/enigma/EnigmaFileReader.java @@ -155,13 +155,14 @@ private static void readClassBody(ColumnFileReader reader, int indent, String sr } String dstNameOrSrcDesc = reader.nextCol(); + String srcDesc = reader.nextCol(); if (dstNameOrSrcDesc == null || dstNameOrSrcDesc.isEmpty()) { errorCollector.addWarning("missing member-name-b/member-desc-a in line "+reader.getLineNumber()); dstNameOrSrcDesc = null; + srcDesc = null; // just to be sure } - String srcDesc = reader.nextCol(); String dstName; if (srcDesc == null) { @@ -222,7 +223,13 @@ private static void readMethod(ColumnFileReader reader, int indent, StringBuilde submitComment(MappedElementKind.METHOD, commentSb, visitor); if (reader.nextCol("ARG")) { // method parameter: ARG - int lvIndex = reader.nextIntCol(); + int lvIndex = -1; + + try { + lvIndex = reader.nextIntCol(false); + } catch (NumberFormatException e) { + // lvIndex remains -1, handled below + } if (lvIndex < 0) { errorCollector.addError("missing/invalid parameter lv-index in line "+reader.getLineNumber()); @@ -231,7 +238,14 @@ private static void readMethod(ColumnFileReader reader, int indent, StringBuilde if (visitor.visitMethodArg(-1, lvIndex, null)) { String dstName = reader.nextCol(); - if (dstName != null && !dstName.isEmpty()) visitor.visitDstName(MappedElementKind.METHOD_ARG, 0, dstName); + + if (dstName != null) { + if (dstName.isEmpty()) { + errorCollector.addWarning("missing var-name-b in line "+reader.getLineNumber()); + } else { + visitor.visitDstName(MappedElementKind.METHOD_ARG, 0, dstName); + } + } readElement(reader, MappedElementKind.METHOD_ARG, indent, commentSb, visitor); } diff --git a/src/main/java/net/fabricmc/mappingio/format/srg/SrgFileReader.java b/src/main/java/net/fabricmc/mappingio/format/srg/SrgFileReader.java index 4f566139..77db699f 100644 --- a/src/main/java/net/fabricmc/mappingio/format/srg/SrgFileReader.java +++ b/src/main/java/net/fabricmc/mappingio/format/srg/SrgFileReader.java @@ -105,6 +105,7 @@ private static void read(ColumnFileReader reader, String sourceNs, String target if (dstName == null || dstName.isEmpty()) { errorCollector.addWarning("missing class-name-b in line "+reader.getLineNumber()); + dstName = null; } visitor.visitDstName(MappedElementKind.CLASS, 0, dstName); diff --git a/src/main/java/net/fabricmc/mappingio/format/srg/TsrgFileReader.java b/src/main/java/net/fabricmc/mappingio/format/srg/TsrgFileReader.java index b1d83cf4..45598c4c 100644 --- a/src/main/java/net/fabricmc/mappingio/format/srg/TsrgFileReader.java +++ b/src/main/java/net/fabricmc/mappingio/format/srg/TsrgFileReader.java @@ -305,7 +305,13 @@ private static void readMethod(ColumnFileReader reader, int dstNsCount, MappingV if (reader.nextCol("static")) { // method is static } else { - int lvIndex = reader.nextIntCol(); + int lvIndex = -1; + + try { + lvIndex = reader.nextIntCol(false); + } catch (NumberFormatException e) { + // lvIndex remains -1, handled below + } if (lvIndex < 0) { errorCollector.addWarning("missing/invalid parameter lv-index in line "+reader.getLineNumber()); diff --git a/src/main/java/net/fabricmc/mappingio/format/tiny/Tiny2FileReader.java b/src/main/java/net/fabricmc/mappingio/format/tiny/Tiny2FileReader.java index 4df873cf..66ecb78e 100644 --- a/src/main/java/net/fabricmc/mappingio/format/tiny/Tiny2FileReader.java +++ b/src/main/java/net/fabricmc/mappingio/format/tiny/Tiny2FileReader.java @@ -46,8 +46,8 @@ public static List getNamespaces(Reader reader) throws IOException { private static List getNamespaces(ColumnFileReader reader) throws IOException { if (!reader.nextCol("tiny") // magic - || reader.nextIntCol() != 2 // major version - || reader.nextIntCol() < 0) { // minor version + || reader.nextIntCol(true) != 2 // major version + || reader.nextIntCol(true) < 0) { // minor version throw new IOException("invalid/unsupported tiny file: no tiny 2 header"); } @@ -72,8 +72,8 @@ public static void read(Reader reader, MappingVisitor visitor, ErrorCollector er private static void read(ColumnFileReader reader, MappingVisitor visitor, ErrorCollector errorCollector) throws IOException { if (!reader.nextCol("tiny") // magic - || reader.nextIntCol() != 2 // major version - || reader.nextIntCol() < 0) { // minor version + || reader.nextIntCol(true) != 2 // major version + || reader.nextIntCol(true) < 0) { // minor version throw new IOException("invalid/unsupported tiny file: no tiny 2 header"); } @@ -203,7 +203,14 @@ private static void readMethod(ColumnFileReader reader, int dstNsCount, boolean while (reader.nextLine(2)) { if (reader.nextCol("p")) { // method parameter: p ... - int lvIndex = reader.nextIntCol(); + int lvIndex = -1; + + try { + lvIndex = reader.nextIntCol(false); + } catch (NumberFormatException e) { + errorCollector.addError("invalid parameter lv-index in line "+reader.getLineNumber()); + continue; + } if (lvIndex < 0) { errorCollector.addWarning("missing/invalid parameter lv-index in line "+reader.getLineNumber()); @@ -224,26 +231,53 @@ private static void readMethod(ColumnFileReader reader, int dstNsCount, boolean readElement(reader, MappedElementKind.METHOD_ARG, dstNsCount, escapeNames, visitor, errorCollector); } } else if (reader.nextCol("v")) { // method variable: v ... - int lvIndex = reader.nextIntCol(); + int lvIndex = -1; + + try { + lvIndex = reader.nextIntCol(false); + } catch (NumberFormatException e) { + errorCollector.addError("invalid variable lv-index in line "+reader.getLineNumber()); + continue; + } if (lvIndex < 0) { errorCollector.addWarning("missing/invalid variable lv-index in line "+reader.getLineNumber()); lvIndex = -1; } - int startOpIdx = reader.nextIntCol(); + int startOpIdx = -1; + + try { + startOpIdx = reader.nextIntCol(false); + } catch (NumberFormatException e) { + errorCollector.addError("invalid variable lv-start-offset in line "+reader.getLineNumber()); + continue; + } if (startOpIdx < 0) { errorCollector.addWarning("missing/invalid variable lv-start-offset in line "+reader.getLineNumber()); startOpIdx = -1; } - int lvtRowIndex = reader.nextIntCol(); - String srcName = reader.nextCol(escapeNames); + int lvtRowIndex = -1; - if (srcName == null) { - errorCollector.addWarning("missing var-name-a column in line "+reader.getLineNumber()); - } else if (srcName.isEmpty()) { + try { + lvtRowIndex = reader.nextIntCol(false); + } catch (NumberFormatException e) { + errorCollector.addError("invalid variable lvt-index in line "+reader.getLineNumber()); + continue; + } + + if (lvtRowIndex < -1) { + errorCollector.addWarning("invalid variable lvt-index in line "+reader.getLineNumber()); + lvtRowIndex = -1; + } + + String srcName = null; + + if (reader.isAtEol()) { + errorCollector.addWarning("missing var-name columns in line "+reader.getLineNumber()); + } else if ((srcName = reader.nextCol(escapeNames)).isEmpty()) { srcName = null; } diff --git a/src/test/java/net/fabricmc/mappingio/read/InvalidContentReadTest.java b/src/test/java/net/fabricmc/mappingio/read/InvalidContentReadTest.java new file mode 100644 index 00000000..c04deb00 --- /dev/null +++ b/src/test/java/net/fabricmc/mappingio/read/InvalidContentReadTest.java @@ -0,0 +1,294 @@ +/* + * Copyright (c) 2023 FabricMC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package net.fabricmc.mappingio.read; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import java.io.IOException; +import java.io.StringReader; +import java.util.List; + +import org.jetbrains.annotations.Nullable; +import org.junit.jupiter.api.Test; + +import net.fabricmc.mappingio.MappedElementKind; +import net.fabricmc.mappingio.MappingReader; +import net.fabricmc.mappingio.NopMappingVisitor; +import net.fabricmc.mappingio.format.ErrorCollector; +import net.fabricmc.mappingio.format.ErrorCollector.ParsingError; +import net.fabricmc.mappingio.format.ErrorCollector.Severity; +import net.fabricmc.mappingio.format.MappingFormat; + +public class InvalidContentReadTest { + private static final String tinyHeader = "v1 source target\n"; + private static final String tiny2Header = "tiny 2 0 source target\n"; + + @Test + public void tinyFile() throws Exception { + MappingFormat format = MappingFormat.TINY_FILE; + + checkThrows(" ", format); + checkWorks(tinyHeader, format); + + checkTinyLine(MappedElementKind.CLASS); + checkTinyLine(MappedElementKind.FIELD); + checkTinyLine(MappedElementKind.METHOD); + } + + private void checkTinyLine(MappedElementKind kind) throws Exception { + MappingFormat format = MappingFormat.TINY_FILE; + String prefix = tinyHeader + kind.name(); + + // No source/target + checkError(prefix, format); + + // Tabs for separation + prefix += "\t"; + checkError(prefix, format); + prefix += "src"; + checkError(prefix, format); + + if (kind == MappedElementKind.FIELD || kind == MappedElementKind.METHOD) { + prefix += "\t"; + checkError(prefix, format); + + prefix += kind == MappedElementKind.FIELD ? "I" : "()V"; + checkError(prefix, format); + + prefix += "\tsrc"; + checkError(prefix, format); + } + + checkWorks(prefix += "\t", format); + checkWorks(prefix += "dst", format); + } + + @Test + public void tinyV2File() throws Exception { + MappingFormat format = MappingFormat.TINY_2_FILE; + + checkThrows(" ", format); + checkWorks(tiny2Header, format); + checkError(tiny2Header + "\t", format); // missing property key + + checkTiny2Line(MappedElementKind.CLASS); + checkTiny2Line(MappedElementKind.FIELD); + checkTiny2Line(MappedElementKind.METHOD); + checkTiny2Line(MappedElementKind.METHOD_ARG); + checkTiny2Line(MappedElementKind.METHOD_VAR); + } + + private void checkTiny2Line(MappedElementKind kind) throws Exception { + MappingFormat format = MappingFormat.TINY_2_FILE; + String prefix = tiny2Header + "c"; + + if (kind != MappedElementKind.CLASS) { + prefix += "\tsrc\t\n\t" + (kind == MappedElementKind.FIELD ? "f" : "m"); + } + + // No source/target + checkError(prefix, format); + + // Tabs for separation + if (kind != MappedElementKind.CLASS) { + checkError(prefix, format); + prefix += "\t"; + checkError(prefix, format); + + prefix += kind == MappedElementKind.FIELD ? "I" : "()V"; + checkError(prefix, format); + } + + prefix += "\t"; + checkError(prefix, format); + prefix += "src"; + + checkWarning(prefix, format); + checkWorks(prefix += "\t", format); + checkWorks(prefix += "dst", format); + checkWorks(prefix += "\n", format); + + if (kind == MappedElementKind.METHOD_ARG) { + checkWarning(prefix += "\t\tp", format); + checkError(prefix += "\t", format); + checkError(prefix + "\t", format); + checkWarning(prefix + "-1", format); + checkWarning(prefix += "0", format); + checkWarning(prefix += "\t", format); + checkWorks(prefix + "\t", format); + checkWarning(prefix += "src", format); + checkWorks(prefix += "\t", format); + checkWorks(prefix += "dst", format); + } else if (kind == MappedElementKind.METHOD_VAR) { + checkWarning(prefix += "\t\tv", format); + checkError(prefix += "\t", format); + checkError(prefix + "\t", format); + checkWarning(prefix + "-1", format); + checkWarning(prefix += "0", format); + checkError(prefix += "\t", format); + checkWarning(prefix + "-1", format); + checkWarning(prefix += "0", format); + checkWorks(prefix + "\t-1\t\t", format); + checkError(prefix += "\t", format); + checkWarning(prefix + "-2\t\t", format); + checkWarning(prefix += "-1", format); + checkWarning(prefix += "\t", format); + checkWorks(prefix + "\t", format); + checkWarning(prefix += "src", format); + checkWorks(prefix += "\t", format); + checkWorks(prefix += "dst", format); + } + } + + @Test + public void enigmaFile() throws Exception { + MappingFormat format = MappingFormat.ENIGMA_FILE; + String prefix = "CLASS"; + + checkError(prefix, format); + checkError(prefix += " ", format); + checkWorks(prefix += "src", format); + checkWorks(prefix += "\n", format); + checkError(prefix += "\tMETHOD", format); + + checkError(prefix + " ", format); + checkWarning(prefix += " src", format); + checkWarning(prefix + " ", format); + checkError(prefix += " ()V\n\t\tARG", format); + + checkError(prefix += " ", format); + checkError(prefix + " ", format); + checkError(prefix + "src", format); + checkWorks(prefix += "0", format); + checkWarning(prefix += " ", format); + } + + @Test + public void srgFile() throws Exception { + MappingFormat format = MappingFormat.SRG_FILE; + + checkSrgLine(MappedElementKind.CLASS, format); + checkSrgLine(MappedElementKind.FIELD, format); + checkSrgLine(MappedElementKind.METHOD, format); + } + + @Test + public void xsrgFile() throws Exception { + MappingFormat format = MappingFormat.XSRG_FILE; + + checkSrgLine(MappedElementKind.CLASS, format); + checkSrgLine(MappedElementKind.FIELD, format); + checkSrgLine(MappedElementKind.METHOD, format); + } + + private void checkSrgLine(MappedElementKind kind, MappingFormat format) throws Exception { + String prefix; + + if (kind == MappedElementKind.CLASS) { + prefix = "CL:"; + } else { + prefix = (kind == MappedElementKind.FIELD ? "FD:" : "MD:"); + } + + // No source/target + checkError(prefix, format); + + // Spaces for separation + prefix += " "; + checkError(prefix, format); + prefix += "src"; + check(prefix, format, true, kind == MappedElementKind.CLASS ? Severity.WARNING : Severity.ERROR); + String suffix = ""; + + if (kind != MappedElementKind.CLASS) { + prefix += "/"; + checkError(prefix, format); + + prefix += "src"; + checkWarning(prefix, format); + + if (kind == MappedElementKind.METHOD || format == MappingFormat.XSRG_FILE) { + prefix += " "; + checkWarning(prefix, format); + + prefix += kind == MappedElementKind.FIELD ? "I" : "()V"; + checkWarning(prefix, format); + } + + prefix += " dst/"; + checkWarning(prefix, format); + + if (kind == MappedElementKind.METHOD) { + suffix += " ()V"; + } else if (format == MappingFormat.XSRG_FILE) { + suffix += " I"; + } + } else { + prefix += " "; + } + + checkWarning(prefix + "" + suffix, format); + checkWorks(prefix + "dst" + suffix, format); + } + + private void checkThrows(String fileContent, MappingFormat format) throws Exception { + assertThrows(IOException.class, () -> checkWorks(fileContent, format)); + } + + private void check(String fileContent, MappingFormat format, boolean shouldError, @Nullable Severity expectedSeverity) throws Exception { + if (!shouldError) { + checkWorks(fileContent, format); + } else { + checkSeverity(fileContent, format, expectedSeverity); + } + } + + private void checkWorks(String fileContent, MappingFormat format) throws Exception { + checkSeverity(fileContent, format, null); + } + + private void checkInfo(String fileContent, MappingFormat format) throws Exception { + checkSeverity(fileContent, format, Severity.INFO); + } + + private void checkWarning(String fileContent, MappingFormat format) throws Exception { + checkSeverity(fileContent, format, Severity.WARNING); + } + + private void checkError(String fileContent, MappingFormat format) throws Exception { + checkSeverity(fileContent, format, Severity.ERROR); + } + + private void checkSeverity(String fileContent, MappingFormat format, @Nullable Severity expectedSeverity) throws Exception { + List errors = read(fileContent, format); + + if (expectedSeverity != null) { + assertFalse(errors.isEmpty(), "Expected error not found"); + assertEquals(expectedSeverity, errors.get(0).getSeverity(), "Wrong error severity"); + } else { + assertEquals(0, errors.size(), "Unexpected error found"); + } + } + + private List read(String fileContent, MappingFormat format) throws Exception { + ErrorCollector errorCollector = ErrorCollector.create(); + MappingReader.read(new StringReader(fileContent), format, new NopMappingVisitor(true), errorCollector); + return errorCollector.getErrors(); + } +}