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

spotbugs report: broken links with inner classes #236

Closed
davewichers opened this issue Jul 3, 2020 · 14 comments
Closed

spotbugs report: broken links with inner classes #236

davewichers opened this issue Jul 3, 2020 · 14 comments
Assignees

Comments

@davewichers
Copy link

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
Copy link

welcome bot commented Jul 3, 2020

Thanks for opening your first issue here! 😃
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
Copy link
Member

Note: charcode of $ = 36 = 0x24

@KengoTODA
Copy link
Member

KengoTODA commented Jul 4, 2020

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
Copy link
Author

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
Copy link
Member

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.

@KengoTODA KengoTODA transferred this issue from spotbugs/spotbugs Jul 5, 2020
@hazendaz
Copy link
Member

@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
Copy link
Author

davewichers commented Aug 24, 2020

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
Copy link
Author

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

@hazendaz
Copy link
Member

hazendaz commented Apr 3, 2022 via email

@davewichers
Copy link
Author

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
Copy link
Member

@davewichers Is this sill an issue?

@davewichers
Copy link
Author

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

@hazendaz
Copy link
Member

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
Copy link
Member

@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.

@hazendaz hazendaz self-assigned this Feb 26, 2023
hazendaz added a commit that referenced this issue Feb 26, 2023
[fix] Resolve inner classes broken links per #236
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants