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

Add option to use latest fingerprints_data.json file #122

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

scottdharvey
Copy link

I wanted the option to use the latest fingerprints_data.json in my applications without having to rebuild the binary with the latest embedded file. Taking #109 into account, I added an option to include the already-embedded file, as well as an option to overwrite the embedded signatures when the app name conflicted.

This is similar to #109 and takes its intentions into account. Where #109 would break existing uses of New(), this will not. It adds a new exported function for this specific use case, and one non-exported function for loading the file.

// NewFromFile creates a new tech detection instance from a file
// this allows using the latest fingerprints without recompiling the code
// loadEmbedded indicates whether to load the embedded fingerprints
// supersede indicates whether to overwrite the embedded fingerprints (if loaded) with the file fingerprints if the app name conflicts
// supersede is only used if loadEmbedded is true
func NewFromFile(filePath string, loadEmbedded, supersede bool) (*Wappalyze, error) {
...
}
wappalyzer.NewFromFile("fingerprints_data.json", false, false)

This allows a user to combine the embedded file with their own custom signatures, or use only their custom signatures. If the user's signatures include one or more app name that is included in the embedded signatures, they can optionally overwrite the embedded version with their custom version with the supersede flag. (this could be changed to overwrite as well)

@matoszz
Copy link

matoszz commented Feb 17, 2025

@scottdharvey I'm not crazy familiar with this codebase (just began using it) but I was curious if there was method(s) / thoughts on merging the embedded signature with a custom provided one rather than an overwrite operation. The use case I was considering was if there was already an app / signature defined (in this case I was trying to figure out why this wasn't detecting the use of Webflow on one of my domains) and thought if you brought your own signature or detection methods that it could be appended to the existing app instead of totally overwriting it.

I realize that you could simply take the defined signature in the embedded file and then add whatever you wanted to it so that when it was overwritten it would include your "total" but figured I'd ask!

Appreciate the work you / projectdiscovery team are doing with these libraries, so useful <3

@scottdharvey
Copy link
Author

scottdharvey commented Feb 19, 2025

@scottdharvey I'm not crazy familiar with this codebase (just began using it) but I was curious if there was method(s) / thoughts on merging the embedded signature with a custom provided one rather than an overwrite operation. The use case I was considering was if there was already an app / signature defined (in this case I was trying to figure out why this wasn't detecting the use of Webflow on one of my domains) and thought if you brought your own signature or detection methods that it could be appended to the existing app instead of totally overwriting it.

I realize that you could simply take the defined signature in the embedded file and then add whatever you wanted to it so that when it was overwritten it would include your "total" but figured I'd ask!

Appreciate the work you / projectdiscovery team are doing with these libraries, so useful <3

@matoszz I am not opposed to the change, but it might make things cluttered unless I introduce opts ...Option to overwrite or merge with the embedded signatures.

I'd be willing to add it if others think it would be the right direction.

@matoszz
Copy link

matoszz commented Feb 19, 2025

@scottdharvey I'm personally a huge fan of functional options parameters, but I'm also not yet a contributor or maintainer of this project so don't want to speak out of turn. While it's not directly relevant to this specific repo (since there weren't actually a ton of options to include) I did recently open a PR on one of my projects which creates wrappers around several of the other projectdiscovery tools (dnsx, subfinder, etc) with functional options parameters if it's at all relevant to you.

If you wanted to limit the scope of your current pull request to what you've already written to get it merged, I'd be willing to take a stab at expanding on the functionality with what I proposed above ^ as well

@scottdharvey
Copy link
Author

@scottdharvey I'm personally a huge fan of functional options parameters, but I'm also not yet a contributor or maintainer of this project so don't want to speak out of turn. While it's not directly relevant to this specific repo (since there weren't actually a ton of options to include) I did recently open a PR on one of my projects which creates wrappers around several of the other projectdiscovery tools (dnsx, subfinder, etc) with functional options parameters if it's at all relevant to you.

If you wanted to limit the scope of your current pull request to what you've already written to get it merged, I'd be willing to take a stab at expanding on the functionality with what I proposed above ^ as well

This is only my 2nd contribution to PD, and my first to this repo, so I don't have any authority either. Personally, I would prefer not to introduce this new NewFromFile function just to turn around and change the variables to call it, breaking any code that uses it in the interim.

I can move supersede to an option and add merge. I think merge would mean we append any signatures from the custom file to the embedded signatures for that app, and if 'description' or some other fields are present, then the embedded description simply gets overwritten by the custom one. If you only need to add potential signatures to match, then the other fields can be left out. Thoughts?

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

Successfully merging this pull request may close these issues.

2 participants