spotbugs / spotbugs-maven-plugin

Maven Mojo Plug-In to generate reports based on the SpotBugs Analyzer
https://spotbugs.github.io/spotbugs-maven-plugin/
Apache License 2.0
69 stars 51 forks source link

spotbugs report: broken links with inner classes #236

Closed davewichers closed 1 year ago

davewichers commented 3 years ago

I believe this is basically the same issue another tool had as documented here: https://issues.apache.org/jira/browse/SUREFIRE-1443

In my project, I noticed warnings like: [warn] [XHTML Sink] Modified invalid anchor name: 'org.owasp.esapi.SecurityConfiguration$Threshold' to 'org.owasp.esapi.SecurityConfigurationThreshold'

But when I looked in the generated spotbugs.html file, I noticed the error on this line that was generated: <h3><a name="org.owasp.esapi.SecurityConfiguration.24Threshold"></a>org.owasp.esapi.SecurityConfiguration$Threshold</h3>

Notice the .24 instead of the $ in the value of the name property. If it generated this: <h3><a name="org.owasp.esapi.SecurityConfiguration$Threshold"></a>org.owasp.esapi.SecurityConfiguration$Threshold</h3>

Then the link to the anchor tag works. To fix this I think you simply need to decode the full classname before putting it's value in the name property. Not sure if that would get rid of this warning message or if that is something else. But this seems to fix the bug in the .html.

I'm using the 4.0.4 version of the spotbugs-maven-plugin to generate the report.

welcome[bot] commented 3 years ago

Thanks for opening your first issue here! :smiley: Please check our contributing guideline. Especially when you report a problem, make sure you share a Minimal, Complete, and Verifiable example to reproduce it in this issue.

KengoTODA commented 3 years ago

Note: charcode of $ = 36 = 0x24

KengoTODA commented 3 years ago

The maven-surefire's fix is related to doxia's usage. SpotBugs does not use doxia, but it could be a good reference.

@davewichers could you tell me which stylesheet (.xsl) you applied? I used default.xsl, fancy.xsl, and fancy-hist.xsl to reproduce in the commit 740999397, but failed.

davewichers commented 3 years ago

I'm not specifying any stylesheet. Just using the default plugin. I am using it with the FindSecBugs plugin if that matters at all. The pom with my configuration is here: https://github.com/kwwall/esapi-java-legacy/tree/misc-cleanup (in this branch). So you should be able to clone that, then run: mvn compile site -DskipTests to reproduce the error messages I'm seeing (there are like 5 of them), and then go to the SpotBugs report generated and see the broken HTML I described.

KengoTODA commented 3 years ago

I run mvn clean site and found that the problem exists not in target/spotbugsXml.xml but target/site/spotbugs.html. Then I think this issue comes from spotbugs-maven-plugin, so I'll transfer this issue to their repo.

hazendaz commented 3 years ago

@davewichers Is it possible you could see about getting a pull request that might fix this issue? I looked at the links but wasn't completely clear on what needed done here to correc this problem.

davewichers commented 3 years ago

I have no idea how to correct this. I just noticed the bug and describe what the correct output should be. I also noticed related discussions on the same problem in other tools, and @KengoTODA decided this issue belongs to this project rather than SpotBugs itself. Basically, there is a step missing somewhere where a String needs to be decoded before being included in the output, as an encoded value of .24 is being output rather than the original $ character.

davewichers commented 2 years ago

Hey - its been 8 months. Any progress on this? I happened to notice it again today :-)

hazendaz commented 2 years ago

Please reconfirm on latest and if possible a PR. This has not been looked at outside of notes here.

Sent from my Verizon, Samsung Galaxy smartphone Get Outlook for Androidhttps://aka.ms/AAb9ysg


From: Dave Wichers @.> Sent: Sunday, April 3, 2022 4:11:24 PM To: spotbugs/spotbugs-maven-plugin @.> Cc: Jeremy Landis @.>; Comment @.> Subject: Re: [spotbugs/spotbugs-maven-plugin] spotbugs report: broken links with inner classes (#236)

Hey - its been 8 months. Any progress on this? I happened to notice it again today :-)

— Reply to this email directly, view it on GitHubhttps://github.com/spotbugs/spotbugs-maven-plugin/issues/236#issuecomment-1086940312, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAHODI2SSS3PL3V56OEV2Q3VDH3OZANCNFSM4OQ2PSQA. You are receiving this because you commented.Message ID: @.***>

davewichers commented 2 years ago

Pull Request? For what? I don't know how to fix this issue. I can confirm I'm using: Spotbugs-maven-plugin 4.6.0.0 and I still have this problem.

hazendaz commented 1 year ago

@davewichers Is this sill an issue?

davewichers commented 1 year ago

Yes - Unless you've fixed it. Do you think you fixed it?

hazendaz commented 1 year ago

Hi @davewichers, likely not fixed, was just checking due to time lapse here. I'm more or less now ready to release. Will take another look at this to see if I can resolve before release. Thanks.

hazendaz commented 1 year ago

@davewichers Resolved this. Issue was a bit different than original posted here in that the 24 still remains and is fine. The actual issue was that $ was removed line above it which prevented click.

Using esapi from roughly point in time you reported this from commit b6f8808e, I was able to produce this data area using existing release. The latest esapi isn't running this report at all so that was reason to drop back to that point to see the warnings and resolve it.

<td>Medium</td></tr></table></section><a name="org.owasp.esapi.SecurityConfigurationThreshold"></a><section>
<h3><a name="org.owasp.esapi.SecurityConfiguration.24Threshold"></a>org.owasp.esapi.SecurityConfiguration$Threshold</h3>

Note line 1 there, its missing $ between SecurityConfiguration and Threshold.

After applying same fix that surefire did and dealing with how groovy handles closures, I was able to get this result.

<td>Medium</td></tr></table></section><a name="org.owasp.esapi.SecurityConfiguration$Threshold"></a><section>
<h3><a name="org.owasp.esapi.SecurityConfiguration.24Threshold"></a>org.owasp.esapi.SecurityConfiguration$Threshold</h3>

Note 24 still exists but the first line has $ now. That $ fix is all that surefire was doing. Now the click works for that one.