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

[jimple] Remove jsp add html and json #4528

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

itsacoderepo
Copy link
Contributor

I would not consider jsp to be config files but code. Also added json and HTML files. The latter usually also does not contain configurations but i would expect it there (currently).

I would not consider jsp to be config files but code. Also added json and HTML files. The latter usually also does not contain configurations but i would expect it there (currently).
@itsacoderepo itsacoderepo requested a review from maltek May 2, 2024 17:32
Copy link
Collaborator

@DavidBakerEffendi DavidBakerEffendi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You may need to also add entries for this under io.joern.x2cpg.passes.frontend.JavaConfigFileCreationPass to have it working e2e

@DavidBakerEffendi DavidBakerEffendi added the jvm Relates to jimple2cpg label May 3, 2024
@maltek
Copy link
Contributor

maltek commented May 3, 2024

Besides the weirdly formatted Java code, JSP will usually contain HTML. At qwiet, we don't use jimple2cpg, but for javasrc2cpg and java2cpg we look at the HTML contained in .jsp config files just as we do with .htm[l] files.

@itsacoderepo
Copy link
Contributor Author

Besides the weirdly formatted Java code, JSP will usually contain HTML.

For me it is ok to keep (or add?) the JSP to the config file node because we might loose (important) information otherwise.

However, the point of JSP having HTML content is opening up a path where we potentially end up adding everything except .class files (for this frontend), because it is also true for PHP, JS, txt.. and all the extensions that we haven't considered. As we know, .war/ear/jar files have weird stuff in it ..

Copy link
Collaborator

@DavidBakerEffendi DavidBakerEffendi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For Jimple this choice is fine I think, but remember to add .html to the filters in io.joern.x2cpg.passes.frontend.JavaConfigFileCreationPass otherwise this won't be picked up. Alternatively, one can add a config test similar to what is in Java.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
jvm Relates to jimple2cpg
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants