Closed ajistrying closed 8 months ago
@ajistrying golangci-lint is failing, fix that and I'll start reviewing :)
@ajistrying golangci-lint is failing, fix that and I'll start reviewing :)
Managed to fix the one warning that I had caused to pop up, but there's a couple that popped up where I didn't touch the code, and I don't necessarily think the warnings are worth pursuing, what are your thoughts? Here they are:
what are your thoughts?
For those ones it's better to build the string using fmt.Sprintf
. Then we could check the lint errors again :)
P.S. We should update the readme accordingly as well
readme updated and linter warnings addressed, hopefully this is what you had in mind!
Have you ever tried the code you've written? Is it working properly? Because it looks like you copied the virustotal part here, imo it can't work
Sorry I should've been more clear, I wanted to see if I was on the right track while working through this. I didn't realize that I instinctively moved the PR over to ready for review, but should have given that CI has been running (it's been a long couple of weeks 😅)
The builtwith implementation is taken from virustotal and I'm not done with it, but I was using the virustotal implementation as a base since I'm guessing some of the same functionality will be used.
Sorry for the misunderstanding!
@edoardottt Now it's really ready for review lol, went through and tested https://api.builtwith.com/domain-api with some test credits and everything looks to be in good shape, here are some screenshots verifying that we're getting back what we need:
(Tested that we were getting back a legit response from Builtwith)
(Cleaned up version)
Sorry for the misunderstanding!
No worries :)
There's a check failing (go build), I'll start reviewing the pr when all the checks are passing
Wanted to bump this :)
This is the only result I get with a valid API key:
===================================================== target: google.com ================ SCANNING SUBDOMAINS ================ Pulling data from BuiltWith google.com.google.com
I'm getting the same response from testing the api using the documented call in the docs with google.com
as well, so unsure of what your expectation was of what subdomains would be coming back from this service. If that's not what was expected, what was your expectation? Also, if this is the incorrect api that I'm using could you point me to the correct api within Builtwith that you believe needs to be used instead?
Trying with the domain builtwith.com
as input I get more results and it seems to work fine. Due to the fact that for now it's not that reliable I think the best option is to merge this PR adding builtwith as subdomain source but commenting the adding phase in the runner file. This means this source will not be used for now. What do you think?
Trying with the domain
builtwith.com
as input I get more results and it seems to work fine. Due to the fact that for now it's not that reliable I think the best option is to merge this PR adding builtwith as subdomain source but commenting the adding phase in the runner file. This means this source will not be used for now. What do you think?
I think that makes sense, unfortunate that it didn't work out as a reliable source, but it happens! I added the comment // Service not fully reliable yet
, is that okay or should I use // Service not working
like the others?
Attempts to add BuiltWith as a subdomain source for the
subdomain
command