Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve handling of relative file paths in report files #2747

Merged
merged 2 commits into from
Sep 24, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 14 additions & 2 deletions cxx-squid/src/main/java/org/sonar/cxx/utils/CxxReportLocation.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
*/
package org.sonar.cxx.utils;

import java.nio.file.Paths;
import java.util.Objects;
import javax.annotation.Nullable;
import org.sonar.api.utils.PathUtils;
Expand All @@ -35,8 +36,19 @@ public class CxxReportLocation {

public CxxReportLocation(@Nullable String file, @Nullable String line, @Nullable String column, String info) {
super();
// normalize file, removing double and single dot path steps => avoids duplicates
this.file = PathUtils.sanitize(file);

// Normalize file using separators in UNIX format, removing double and single dot path steps. This is to avoid
// duplicates in the issue containers because they are using the file as key. PathUtils.sanitize uses
// FilenameUtils.normalize internally, relative paths starting with a double dot will cause that path segment
// and the one before to be removed. If the double dot has no parent path segment to work with, null is returned.
// null would mean 'project issue' which is wrong in this context. To avoid this we extract the filename to
// generate at least a meningful error message.
var normalized = PathUtils.sanitize(file);
if (normalized == null && (file != null && !file.isBlank())) {
normalized = Paths.get(file).getFileName().toString();
}

this.file = normalized;
this.line = line;
this.column = column;
this.info = info;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
/*
* C++ Community Plugin (cxx plugin)
* Copyright (C) 2010-2023 SonarOpenCommunity
* http://github.com/SonarOpenCommunity/sonar-cxx
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 3 of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public License
* along with this program; if not, write to the Free Software Foundation,
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/
package org.sonar.cxx.utils;

import static org.assertj.core.api.Assertions.*;
import org.junit.jupiter.api.Test;

public class CxxReportLocationTest {

@Test
void testConstructor() {
var loc0 = new CxxReportLocation(null, null, null, "info");
assertThat(loc0.getFile()).isNull();
assertThat(loc0.getLine()).isNull();
assertThat(loc0.getColumn()).isNull();
assertThat(loc0.getInfo()).isEqualTo("info");
assertThat(loc0.toString()).isEqualTo("CxxReportLocation [file=null, line=null, column=null, info=info]");

var loc1 = new CxxReportLocation("file", "line", "column", "info");
assertThat(loc1.getFile()).isEqualTo("file");
assertThat(loc1.getLine()).isEqualTo("line");
assertThat(loc1.getColumn()).isEqualTo("column");
assertThat(loc1.getInfo()).isEqualTo("info");
assertThat(loc1.toString()).isEqualTo("CxxReportLocation [file=file, line=line, column=column, info=info]");
}

@Test
void testPathSanitize() {
var loc = new CxxReportLocation("file", null, null, "");

loc = new CxxReportLocation("/dir/File", null, null, "");
assertThat(loc.getFile()).isEqualTo("/dir/File");
loc = new CxxReportLocation("/dir/./File", null, null, "");
assertThat(loc.getFile()).isEqualTo("/dir/File");
loc = new CxxReportLocation("/dir/../File", null, null, "");
assertThat(loc.getFile()).isEqualTo("/File");
loc = new CxxReportLocation("dir/File", null, null, "");
assertThat(loc.getFile()).isEqualTo("dir/File");
loc = new CxxReportLocation("./dir/File", null, null, "");
assertThat(loc.getFile()).isEqualTo("dir/File");
loc = new CxxReportLocation("../dir/File", null, null, "");
assertThat(loc.getFile()).isEqualTo("File");
loc = new CxxReportLocation("../../File", null, null, "");
assertThat(loc.getFile()).isEqualTo("File");

loc = new CxxReportLocation("c:\\dir\\file.ext", null, null, "");
assertThat(loc.getFile()).isEqualTo("c:/dir/file.ext");
loc = new CxxReportLocation("C:\\dir\\.\\file.ext", null, null, "");
assertThat(loc.getFile()).isEqualTo("C:/dir/file.ext");
loc = new CxxReportLocation("c:\\dir\\..\\file.ext", null, null, "");
assertThat(loc.getFile()).isEqualTo("c:/file.ext");
loc = new CxxReportLocation("dir\\file.ext", null, null, "");
assertThat(loc.getFile()).isEqualTo("dir/file.ext");
loc = new CxxReportLocation(".\\dir\\file.ext", null, null, "");
assertThat(loc.getFile()).isEqualTo("dir/file.ext");
loc = new CxxReportLocation("..\\dir\\file.ext", null, null, "");
assertThat(loc.getFile()).isEqualTo("file.ext");
loc = new CxxReportLocation("..\\..\\file.ext", null, null, "");
assertThat(loc.getFile()).isEqualTo("file.ext");
}
}
Loading