-
Notifications
You must be signed in to change notification settings - Fork 35
Extend webserver collection #264
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
base: main
Are you sure you want to change the base?
Conversation
What version should I put at line 873? See question marks. |
acquire/acquire.py
Outdated
def get_spec_additions(cls, target: Target, cli_args: argparse.Namespace) -> Iterator[tuple]: | ||
spec = set() | ||
|
||
for subclass in WebserverPlugin.__subclasses__(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that this is slightly dangerous because of fox-it/dissect.target#1015 and consequently fox-it/dissect.target#1232.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, did not know about those bugs. What's the way to go here? Wait for these bugs to get fixed or just hardcode the plugins that we have now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the time being maybe hardcode it. The long term plan is to replace the "acquire modules" with the _get_paths()
functionality, so that there's a single source of truth for all required files, instead of acquire
and dissect.target
slowly diverging because they both maintain different lists of files to collect/parse.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good to know! I'll try to keep that in mind for future PRs.
Hardcoded the classes for now.
Any news on this? |
@Schamper do you have some time to take a second look at this? |
I'd like @twiggler to have an opinion too. |
closes #139
Testing on VMs with IIS, Nginx and Caddy seems to work fine with an adjusted version of
dissect.target
#1287.The issue specifies that tests should be written. Do we still want that, even with the tests that are already there in
dissect.target
?