From: Julien HENRY Date: Fri, 20 Feb 2015 14:43:13 +0000 (+0100) Subject: SONAR-6175 Validate that offsets provided for highlighting and symbol reference are val X-Git-Tag: 5.1-RC1~131 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=4434c2ae06e098bb3dbf1aa21edff2a6857699e0;p=sonarqube.git SONAR-6175 Validate that offsets provided for highlighting and symbol reference are val --- diff --git a/plugins/sonar-xoo-plugin/src/test/java/org/sonar/xoo/lang/SyntaxHighlightingSensorTest.java b/plugins/sonar-xoo-plugin/src/test/java/org/sonar/xoo/lang/SyntaxHighlightingSensorTest.java index 9b9a7baca05..d31724c73e8 100644 --- a/plugins/sonar-xoo-plugin/src/test/java/org/sonar/xoo/lang/SyntaxHighlightingSensorTest.java +++ b/plugins/sonar-xoo-plugin/src/test/java/org/sonar/xoo/lang/SyntaxHighlightingSensorTest.java @@ -66,7 +66,7 @@ public class SyntaxHighlightingSensorTest { public void testExecution() throws IOException { File symbol = new File(baseDir, "src/foo.xoo.highlighting"); FileUtils.write(symbol, "1:4:k\n12:15:cppd\n\n#comment"); - DefaultInputFile inputFile = new DefaultInputFile("foo", "src/foo.xoo").setLanguage("xoo"); + DefaultInputFile inputFile = new DefaultInputFile("foo", "src/foo.xoo").setLanguage("xoo").setLastValidOffset(100); context.fileSystem().add(inputFile); sensor.execute(context); diff --git a/sonar-batch/src/main/java/org/sonar/batch/scan/filesystem/FileMetadata.java b/sonar-batch/src/main/java/org/sonar/batch/scan/filesystem/FileMetadata.java index 1bb1a2c4b3e..8a0f242a9a3 100644 --- a/sonar-batch/src/main/java/org/sonar/batch/scan/filesystem/FileMetadata.java +++ b/sonar-batch/src/main/java/org/sonar/batch/scan/filesystem/FileMetadata.java @@ -200,6 +200,7 @@ public class FileMetadata implements BatchComponent { private static class LineOffsetCounter extends CharHandler { private int currentOriginalOffset = 0; private List originalLineOffsets = new ArrayList(); + private int lastValidOffset = 0; public LineOffsetCounter() { originalLineOffsets.add(0); @@ -215,10 +216,19 @@ public class FileMetadata implements BatchComponent { originalLineOffsets.add(currentOriginalOffset); } + @Override + void eof() { + lastValidOffset = currentOriginalOffset; + } + public List getOriginalLineOffsets() { return originalLineOffsets; } + public int getLastValidOffset() { + return lastValidOffset; + } + } /** @@ -236,6 +246,7 @@ public class FileMetadata implements BatchComponent { scanFile(file, encoding, lineCounter, fileHashComputer); } return new Metadata(lineCounter.lines(), lineCounter.nonBlankLines(), fileHashComputer.getHash(), lineOffsetCounter.getOriginalLineOffsets(), + lineOffsetCounter.getLastValidOffset(), lineCounter.isEmpty()); } @@ -288,14 +299,16 @@ public class FileMetadata implements BatchComponent { final int nonBlankLines; final String hash; final int[] originalLineOffsets; + final int lastValidOffset; final boolean empty; - private Metadata(int lines, int nonBlankLines, String hash, List originalLineOffsets, boolean empty) { + private Metadata(int lines, int nonBlankLines, String hash, List originalLineOffsets, int lastValidOffset, boolean empty) { this.lines = lines; this.nonBlankLines = nonBlankLines; this.hash = hash; this.empty = empty; this.originalLineOffsets = Ints.toArray(originalLineOffsets); + this.lastValidOffset = lastValidOffset; } } diff --git a/sonar-batch/src/main/java/org/sonar/batch/scan/filesystem/InputFileBuilder.java b/sonar-batch/src/main/java/org/sonar/batch/scan/filesystem/InputFileBuilder.java index 56b8319ac44..2077e91d578 100644 --- a/sonar-batch/src/main/java/org/sonar/batch/scan/filesystem/InputFileBuilder.java +++ b/sonar-batch/src/main/java/org/sonar/batch/scan/filesystem/InputFileBuilder.java @@ -107,6 +107,7 @@ class InputFileBuilder { FileMetadata.Metadata metadata = fileMetadata.read(inputFile.file(), fs.encoding()); inputFile.setLines(metadata.lines); + inputFile.setLastValidOffset(metadata.lastValidOffset); result.setNonBlankLines(metadata.nonBlankLines); result.setHash(metadata.hash); diff --git a/sonar-batch/src/main/java/org/sonar/batch/sensor/DefaultSensorStorage.java b/sonar-batch/src/main/java/org/sonar/batch/sensor/DefaultSensorStorage.java index f91d3cdb25a..cf2930a72fe 100644 --- a/sonar-batch/src/main/java/org/sonar/batch/sensor/DefaultSensorStorage.java +++ b/sonar-batch/src/main/java/org/sonar/batch/sensor/DefaultSensorStorage.java @@ -214,8 +214,8 @@ public class DefaultSensorStorage implements SensorStorage { public void store(org.sonar.api.batch.sensor.dependency.Dependency dep) { BatchResource fromBatchResource = resourceCache.get(dep.fromKey()); BatchResource toBatchResource = resourceCache.get(dep.toKey()); - Preconditions.checkNotNull(fromBatchResource, "Unable to find from resource " + dep.fromKey()); - Preconditions.checkNotNull(toBatchResource, "Unable to find from resource " + dep.toKey()); + Preconditions.checkNotNull(fromBatchResource, "Unable to find origin resource " + dep.fromKey()); + Preconditions.checkNotNull(toBatchResource, "Unable to find destination resource " + dep.toKey()); File fromResource = (File) fromBatchResource.resource(); File toResource = (File) toBatchResource.resource(); if (sonarIndex.getEdge(fromResource, toResource) != null) { diff --git a/sonar-batch/src/test/java/org/sonar/batch/mediumtest/highlighting/HighlightingMediumTest.java b/sonar-batch/src/test/java/org/sonar/batch/mediumtest/highlighting/HighlightingMediumTest.java index 9b95241d26e..42f7bfc8330 100644 --- a/sonar-batch/src/test/java/org/sonar/batch/mediumtest/highlighting/HighlightingMediumTest.java +++ b/sonar-batch/src/test/java/org/sonar/batch/mediumtest/highlighting/HighlightingMediumTest.java @@ -19,6 +19,7 @@ */ package org.sonar.batch.mediumtest.highlighting; +import com.google.common.base.Strings; import com.google.common.collect.ImmutableMap; import org.apache.commons.io.FileUtils; import org.junit.After; @@ -102,8 +103,8 @@ public class HighlightingMediumTest { File xooFile = new File(srcDir, "sample.xoo"); File xoohighlightingFile = new File(srcDir, "sample.xoo.highlighting"); - FileUtils.write(xooFile, "Sample xoo\ncontent"); int chunkSize = 100000; + FileUtils.write(xooFile, Strings.repeat("a", chunkSize)); StringBuilder sb = new StringBuilder(16 * chunkSize); for (int i = 0; i < chunkSize; i++) { sb.append(i).append(":").append(i + 1).append(":s\n"); diff --git a/sonar-batch/src/test/java/org/sonar/batch/mediumtest/issues/ChecksMediumTest.java b/sonar-batch/src/test/java/org/sonar/batch/mediumtest/issues/ChecksMediumTest.java index 5d1a3838f36..5cc57070ce5 100644 --- a/sonar-batch/src/test/java/org/sonar/batch/mediumtest/issues/ChecksMediumTest.java +++ b/sonar-batch/src/test/java/org/sonar/batch/mediumtest/issues/ChecksMediumTest.java @@ -65,7 +65,7 @@ public class ChecksMediumTest { srcDir.mkdir(); File xooFile = new File(srcDir, "sample.xoo"); - FileUtils.write(xooFile, "foo"); + FileUtils.write(xooFile, "foo\nbar"); TaskResult result = tester.newTask() .properties(ImmutableMap.builder() diff --git a/sonar-batch/src/test/java/org/sonar/batch/scan/filesystem/FileMetadataTest.java b/sonar-batch/src/test/java/org/sonar/batch/scan/filesystem/FileMetadataTest.java index b05c9cc414c..0bd065fc17b 100644 --- a/sonar-batch/src/test/java/org/sonar/batch/scan/filesystem/FileMetadataTest.java +++ b/sonar-batch/src/test/java/org/sonar/batch/scan/filesystem/FileMetadataTest.java @@ -54,6 +54,7 @@ public class FileMetadataTest { assertThat(metadata.nonBlankLines).isEqualTo(0); assertThat(metadata.hash).isNotEmpty(); assertThat(metadata.originalLineOffsets).containsOnly(0); + assertThat(metadata.lastValidOffset).isEqualTo(0); assertThat(metadata.empty).isTrue(); } @@ -67,6 +68,7 @@ public class FileMetadataTest { assertThat(metadata.nonBlankLines).isEqualTo(3); assertThat(metadata.hash).isEqualTo(md5Hex("foo\nbar\nbaz")); assertThat(metadata.originalLineOffsets).containsOnly(0, 5, 10); + assertThat(metadata.lastValidOffset).isEqualTo(13); assertThat(metadata.empty).isFalse(); } @@ -115,6 +117,7 @@ public class FileMetadataTest { assertThat(metadata.nonBlankLines).isEqualTo(3); assertThat(metadata.hash).isEqualTo(md5Hex("foo\nbar\nbaz")); assertThat(metadata.originalLineOffsets).containsOnly(0, 4, 8); + assertThat(metadata.lastValidOffset).isEqualTo(11); } @Test @@ -127,6 +130,7 @@ public class FileMetadataTest { assertThat(metadata.nonBlankLines).isEqualTo(3); assertThat(metadata.hash).isEqualTo(md5Hex("foo\nbar\nbaz\n")); assertThat(metadata.originalLineOffsets).containsOnly(0, 4, 8, 12); + assertThat(metadata.lastValidOffset).isEqualTo(12); } @Test diff --git a/sonar-batch/src/test/java/org/sonar/batch/sensor/DefaultSensorStorageTest.java b/sonar-batch/src/test/java/org/sonar/batch/sensor/DefaultSensorStorageTest.java index 157bd38a20a..b65d7e8050c 100644 --- a/sonar-batch/src/test/java/org/sonar/batch/sensor/DefaultSensorStorageTest.java +++ b/sonar-batch/src/test/java/org/sonar/batch/sensor/DefaultSensorStorageTest.java @@ -176,7 +176,7 @@ public class DefaultSensorStorageTest { @Test public void shouldAddIssueOnFile() { - InputFile file = new DefaultInputFile("foo", "src/Foo.php"); + InputFile file = new DefaultInputFile("foo", "src/Foo.php").setLines(4); ArgumentCaptor argumentCaptor = ArgumentCaptor.forClass(Issue.class); diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/batch/fs/internal/DefaultInputFile.java b/sonar-plugin-api/src/main/java/org/sonar/api/batch/fs/internal/DefaultInputFile.java index cebd7c410f8..c639559c60e 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/batch/fs/internal/DefaultInputFile.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/batch/fs/internal/DefaultInputFile.java @@ -43,6 +43,7 @@ public class DefaultInputFile implements InputFile, Serializable { private Status status; private int lines; private Charset charset; + private int lastValidOffset; public DefaultInputFile(String moduleKey, String relativePath) { this.moduleKey = moduleKey; @@ -144,6 +145,15 @@ public class DefaultInputFile implements InputFile, Serializable { return this; } + public int lastValidOffset() { + return lastValidOffset; + } + + public DefaultInputFile setLastValidOffset(int lastValidOffset) { + this.lastValidOffset = lastValidOffset; + return this; + } + @Override public boolean equals(Object o) { if (this == o) { diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/batch/sensor/highlighting/internal/DefaultHighlighting.java b/sonar-plugin-api/src/main/java/org/sonar/api/batch/sensor/highlighting/internal/DefaultHighlighting.java index 9849c058335..ca1a0b4a355 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/batch/sensor/highlighting/internal/DefaultHighlighting.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/batch/sensor/highlighting/internal/DefaultHighlighting.java @@ -23,6 +23,7 @@ import com.google.common.base.Preconditions; import com.google.common.collect.Ordering; import com.google.common.collect.Sets; import org.sonar.api.batch.fs.InputFile; +import org.sonar.api.batch.fs.internal.DefaultInputFile; import org.sonar.api.batch.sensor.highlighting.NewHighlighting; import org.sonar.api.batch.sensor.highlighting.TypeOfText; import org.sonar.api.batch.sensor.internal.DefaultStorable; @@ -91,12 +92,21 @@ public class DefaultHighlighting extends DefaultStorable implements NewHighlight @Override public DefaultHighlighting highlight(int startOffset, int endOffset, TypeOfText typeOfText) { Preconditions.checkState(inputFile != null, "Call onFile() first"); + int maxValidOffset = ((DefaultInputFile) inputFile).lastValidOffset(); + checkOffset(startOffset, maxValidOffset, "startOffset"); + checkOffset(endOffset, maxValidOffset, "endOffset"); + Preconditions.checkArgument(startOffset < endOffset, "startOffset (" + startOffset + ") should be < endOffset (" + endOffset + ") for file " + inputFile + "."); SyntaxHighlightingRule syntaxHighlightingRule = SyntaxHighlightingRule.create(startOffset, endOffset, typeOfText); this.syntaxHighlightingRuleSet.add(syntaxHighlightingRule); return this; } + private void checkOffset(int offset, int maxValidOffset, String label) { + Preconditions.checkArgument(offset >= 0 && offset <= maxValidOffset, "Invalid " + label + " " + offset + ". Should be >= 0 and <= " + maxValidOffset + + " for file " + inputFile); + } + @Override protected void doSave() { Preconditions.checkState(inputFile != null, "Call onFile() first"); diff --git a/sonar-plugin-api/src/test/java/org/sonar/api/batch/sensor/highlighting/internal/DefaultHighlightingTest.java b/sonar-plugin-api/src/test/java/org/sonar/api/batch/sensor/highlighting/internal/DefaultHighlightingTest.java index 55900838bc6..4cf1225e27b 100644 --- a/sonar-plugin-api/src/test/java/org/sonar/api/batch/sensor/highlighting/internal/DefaultHighlightingTest.java +++ b/sonar-plugin-api/src/test/java/org/sonar/api/batch/sensor/highlighting/internal/DefaultHighlightingTest.java @@ -45,7 +45,7 @@ public class DefaultHighlightingTest { public void setUpSampleRules() { DefaultHighlighting highlightingDataBuilder = new DefaultHighlighting() - .onFile(new DefaultInputFile("foo", "src/Foo.java")) + .onFile(new DefaultInputFile("foo", "src/Foo.java").setLastValidOffset(100)) .highlight(0, 10, COMMENT) .highlight(10, 12, KEYWORD) .highlight(24, 38, KEYWORD) @@ -71,7 +71,7 @@ public class DefaultHighlightingTest { @Test public void should_suport_overlapping() throws Exception { new DefaultHighlighting(mock(SensorStorage.class)) - .onFile(new DefaultInputFile("foo", "src/Foo.java")) + .onFile(new DefaultInputFile("foo", "src/Foo.java").setLastValidOffset(100)) .highlight(0, 15, KEYWORD) .highlight(8, 12, CPP_DOC) .save(); @@ -83,10 +83,46 @@ public class DefaultHighlightingTest { throwable.expectMessage("Cannot register highlighting rule for characters from 8 to 15 as it overlaps at least one existing rule"); new DefaultHighlighting(mock(SensorStorage.class)) - .onFile(new DefaultInputFile("foo", "src/Foo.java")) + .onFile(new DefaultInputFile("foo", "src/Foo.java").setLastValidOffset(100)) .highlight(0, 10, KEYWORD) .highlight(8, 15, KEYWORD) .save(); } + @Test + public void should_prevent_invalid_offset() throws Exception { + throwable.expect(IllegalArgumentException.class); + throwable.expectMessage("Invalid endOffset 15. Should be >= 0 and <= 10 for file [moduleKey=foo, relative=src/Foo.java, basedir=null]"); + + new DefaultHighlighting(mock(SensorStorage.class)) + .onFile(new DefaultInputFile("foo", "src/Foo.java").setLastValidOffset(10)) + .highlight(0, 10, KEYWORD) + .highlight(8, 15, KEYWORD) + .save(); + } + + @Test + public void positive_offset() throws Exception { + throwable.expect(IllegalArgumentException.class); + throwable.expectMessage("Invalid startOffset -8. Should be >= 0 and <= 10 for file [moduleKey=foo, relative=src/Foo.java, basedir=null]"); + + new DefaultHighlighting(mock(SensorStorage.class)) + .onFile(new DefaultInputFile("foo", "src/Foo.java").setLastValidOffset(10)) + .highlight(0, 10, KEYWORD) + .highlight(-8, 15, KEYWORD) + .save(); + } + + @Test + public void should_prevent_invalid_offset_order() throws Exception { + throwable.expect(IllegalArgumentException.class); + throwable.expectMessage("startOffset (18) should be < endOffset (15) for file [moduleKey=foo, relative=src/Foo.java, basedir=null]."); + + new DefaultHighlighting(mock(SensorStorage.class)) + .onFile(new DefaultInputFile("foo", "src/Foo.java").setLastValidOffset(20)) + .highlight(0, 10, KEYWORD) + .highlight(18, 15, KEYWORD) + .save(); + } + } diff --git a/sonar-plugin-api/src/test/java/org/sonar/api/batch/sensor/internal/SensorContextTesterTest.java b/sonar-plugin-api/src/test/java/org/sonar/api/batch/sensor/internal/SensorContextTesterTest.java index 92a3e262fc1..0be267797b7 100644 --- a/sonar-plugin-api/src/test/java/org/sonar/api/batch/sensor/internal/SensorContextTesterTest.java +++ b/sonar-plugin-api/src/test/java/org/sonar/api/batch/sensor/internal/SensorContextTesterTest.java @@ -87,12 +87,12 @@ public class SensorContextTesterTest { assertThat(tester.issues("foo:src/Foo.java")).isEmpty(); assertThat(tester.allIssues()).isEmpty(); tester.newIssue() - .onFile(new DefaultInputFile("foo", "src/Foo.java")) + .onFile(new DefaultInputFile("foo", "src/Foo.java").setLines(10)) .forRule(RuleKey.of("repo", "rule")) .atLine(1) .save(); tester.newIssue() - .onFile(new DefaultInputFile("foo", "src/Foo.java")) + .onFile(new DefaultInputFile("foo", "src/Foo.java").setLines(10)) .forRule(RuleKey.of("repo", "rule")) .atLine(3) .save(); @@ -144,7 +144,7 @@ public class SensorContextTesterTest { public void testHighlighting() { assertThat(tester.highlightingTypeFor("foo:src/Foo.java", 3)).isEmpty(); tester.newHighlighting() - .onFile(new DefaultInputFile("foo", "src/Foo.java")) + .onFile(new DefaultInputFile("foo", "src/Foo.java").setLastValidOffset(100)) .highlight(0, 4, TypeOfText.ANNOTATION) .highlight(8, 10, TypeOfText.CONSTANT) .highlight(9, 10, TypeOfText.COMMENT)