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.

Sign up for GitHub

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

Jump to bottom

added indented ast-node-printer #98

Closed
wants to merge 12 commits into from
Closed

added indented ast-node-printer #98

wants to merge 12 commits into from

Conversation

psyirius
Copy link
Collaborator

@psyirius psyirius commented Mar 28, 2023 •
edited
Loading

Addresses: #42

@psyirius psyirius requested a review from AngeloD2022 March 28, 2023 14:46
@psyirius psyirius changed the title added node indented printer added indented ast-node-printer Mar 28, 2023
@AngeloD2022
Copy link
Owner

awesome, I'll check this out later today!

@AngeloD2022 AngeloD2022 changed the base branch from master to development March 28, 2023 22:36
@AngeloD2022
Copy link
Owner

Nice work. Can we add a CLI flag that enables this?

@AngeloD2022 AngeloD2022 added the enhancement New feature or request label Mar 28, 2023
@psyirius
Copy link
Collaborator Author

psyirius commented Mar 29, 2023 •
edited
Loading

Nice work. Can we add a CLI flag that enables this?

Yeah. I'm integrating a new cli parser to make the cli easier to use.
We can roll it out with that change.

BTW, if you got the reference file for #82
mail me.

@psyirius psyirius mentioned this pull request Mar 29, 2023
Open
@AngeloD2022
Copy link
Owner

I'll get those files to you asap

@psyirius
Copy link
Collaborator Author

psyirius commented Mar 29, 2023 •
edited
Loading

Nice work. Can we add a CLI flag that enables this?

DONE!

image

image

@AngeloD2022
Copy link
Owner

awesome! I'll check this out!

@AngeloD2022
Copy link
Owner

So I did a checkout and it doesn't give me the --ast or --verbose options. did you commit those yet?

@psyirius
Copy link
Collaborator Author

So I did a checkout and it doesn't give me the --ast or --verbose options. did you commit those yet?

yes. lemme double check it

@psyirius
Copy link
Collaborator Author

psyirius commented Mar 29, 2023 •
edited
Loading

Yes i am.

@AngeloD2022
Copy link
Owner

I'm an idiot and forgot to git pull haha. my bad

@psyirius
Copy link
Collaborator Author

I'm an idiot and forgot to git pull haha. my bad

pull again.
pushed some more changes.

AngeloD2022
AngeloD2022 reviewed Mar 29, 2023
View reviewed changes
src/jsxer/jsxer.cpp
header += "* Jsxblind Deobfuscation Enabled (EXPERIMENTAL)\n";
header += "* Options:\n";
if (options & kDO_Unblind) {
header += "* \tJsxblind Deobfuscation (EXPERIMENTAL)\n";
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
header += "* \tJsxblind Deobfuscation (EXPERIMENTAL)\n";
header += "* Jsxblind Deobfuscation (EXPERIMENTAL)\n";

Looks kind of strange with a tab there

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

it's there to show a list of options enabled which reflects changes in the decompilation.

Copy link
Owner

Choose a reason for hiding this comment

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

ohhh I didn't see the Options: before it! my bad.

AngeloD2022
AngeloD2022 reviewed Mar 29, 2023
View reviewed changes
src/cli/main.cpp
int options = Options::kDO_None;

if (unblind) {
options |= Options::kDO_Unblind;
Copy link
Owner

Choose a reason for hiding this comment

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

Should we add back a log that alerts if JsxBlind deobfuscation is enabled?

[!] JsxBlind Deobfuscation Enabled

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yeah

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

log right after the cli flag parsed

@AngeloD2022
Copy link
Owner

Looks like there is a circular dependency between jsxer.h/cpp (depends on Reader from reader.h/cpp, which depends on Options from jsxer.h/cpp)

@AngeloD2022 AngeloD2022 marked this pull request as draft May 24, 2023 05:24
@AngeloD2022 AngeloD2022 linked an issue May 24, 2023 that may be closed by this pull request
FeatureRequest: Dump the AST (with a cli flag) #42
Open
@AngeloD2022 AngeloD2022 deleted the branch AngeloD2022:development May 5, 2024 01:00
@AngeloD2022 AngeloD2022 closed this May 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AngeloD2022 AngeloD2022 AngeloD2022 left review comments

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

FeatureRequest: Dump the AST (with a cli flag)
2 participants
@psyirius @AngeloD2022