Improve handling of relative file paths in report files

[guwirth]

• Up to know we were stating in our Wiki: Relative paths in report files are always relative to the project base directory. Start relative paths always with .\ on Windows or ./ on Linux.
• relative paths starting with '..' are resolved to null. "No file" results in adding an issue on project level (project issue). In such a case we replace null with the filename now. This leads at least to an error message and the issue is not added to the project because typically no such indexed file exists.
• The real problem is that tools generate reports with relative paths without defining which base directory is referred to. This problem remains and must be solved via the CI/CD and tool configuration.
• close Relative paths with double dot at the beginning (..\) fail in reports #2741

---

This change is 

[cmorve-te]

The problem still exists when the file starts with ../../ (which I have seen).

[guwirth]

Hello @cmorve-te,

thanks for your test. Can you provide a report file to reproduce the issue. With the unit tests it works? Like to understand what’s different.

Regards,

[cmorve-te]

I guess anything starting with ../../ will fail. Which makes sense, it gets converted to ./../XXX, which is equivalent to ../XXX, which was the original problem.

--- a/cxx-squid/src/test/java/org/sonar/cxx/utils/CxxReportLocationTest.java
+++ b/cxx-squid/src/test/java/org/sonar/cxx/utils/CxxReportLocationTest.java
@@ -57,6 +57,8 @@ public class CxxReportLocationTest {
     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, "");^M
+    assertThat(loc.getFile()).isEqualTo("dir/File");^M
 
     loc = new CxxReportLocation("c:\\dir\\file.ext", null, null, "");
     assertThat(loc.getFile()).isEqualTo("c:/dir/file.ext");

fails with

org.opentest4j.AssertionFailedError: 

expected: "dir/File"
 but was: null
	at org.sonar.cxx.utils.CxxReportLocationTest.testPathSanitize(CxxReportLocationTest.java:61)

Honestly, not even sure what I would write in the assertion. ../../XXX isEqualTo XXX start to be forced.
I guess the best solution would be making anything starting with .. relative to sonar.projectBaseDir. But I don't really mind, in my specific case the path is wrong anyway. The only important thing is that it doesn't end up being null/a "project" issue.

[guwirth]

Hi @cmorve-te,

you are right, I was not testing with relative path with more than one level like ../../file. This will fail again.

I guess the best solution would be making anything starting with .. relative to sonar.projectBaseDir

This can also fail: In case projectBaseDir is /home/sample and path in report file ../../../file it's the same again.

In the meantime I tend to simply add the file name in case PathUtils.sanitize(file) returns null. This way at least no project issue is created and the error message remains helpful.

What do you think?

[cmorve-te]

This can also fail: In case projectBaseDir is /home/sample and path in report file ../../../file it's the same again.

Indeed.

In the meantime I tend to simply add the file name in case PathUtils.sanitize(file) returns null. This way at least no project issue is created and the error message remains helpful.

That would work for me, but can you just add the filename?
My understanding was that you need an "InputFile" (https://github.com/SonarOpenCommunity/sonar-cxx/blob/master/cxx-sensors/src/main/java/org/sonar/cxx/sensors/utils/CxxReportSensor.java#L70), that must exist in the "context.fileSystem()". That just a string to a non-existing path would not be accepted by the Sonar API.

In general, that's something I would like from Sonar. Instead of hoping the warnings don't point to an (existing) file that's not in sonar.sources, it would be nice if any file a warning points to would be automatically added to the list of sources (but not for coverage purposes!) and I could see it from SonarQube. Even if it's a system header, I don't care too much about that code, but having it SonarQube could help understanding the warnings.

So your "simply add the file name" is probably the best option. If not possible, I guess I would just fallback to just drop any warnings that resolve to a null file in the same way they are dropped if the resolve to a valid file not in sonar.sources.

[cmorve-te]

Oh, you meant only the file, without any path, and only inside the CxxReportLocation constructor!
Hopefully everybody has his sources under src, fine with me.

[guwirth]

@cmorve-te maybe you can give this a try: https://github.com/SonarOpenCommunity/sonar-cxx/actions/runs/10972510569/artifacts/1961586003

In case result is null try with filename only. This should avoid project issue generation and should give you a warning in the LOG file.

[guwirth]

Hi @cmorve-te, like to release V2.1.3 and add this one. Any feedback?

[cmorve-te]

Verified with my real output, it's all fine now.

Again, unless the file name happens to match the name of a real file in the root of the project, I guess. My project doesn't have source files in the root of the project at all.

[guwirth]

@cmorve-te thanks for testing I know this is not a perfect solution but an improvement.
Solution in release available: https://github.com/SonarOpenCommunity/sonar-cxx/releases/tag/cxx-2.1.3