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 code to support %DOMAIN% tag replacer #1371

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

Conversation

nrathaus
Copy link

Description

What will it do?

If this PR will fix an issue, please address it:
#1313

if it finds a %DOMAIN% tag inside the dictionary, it will replace it with the hostname field of the URL - if multiple URLs are provided it will replace it multiple times

@maurosoria
Copy link
Owner

Hi! Glad to meet you @nrathaus ! How you doing?

I like it but I wonder, for example if type www.example.com:

  • Shouldn't try example.com?
  • Or just example?

For example, given the case "%DOMAIN%.zip":

@nrathaus
Copy link
Author

@maurosoria I added the iterator so that it will go through the components of the domain

Let me know if this is what you were expecting

@maurosoria
Copy link
Owner

Hi again!

I see you got me but there are 3 issues with your commit:

  • What happens when a line has both %EXT% or %DOMAIN% in the same line (or any other future tag)? I see in this commit you only allow one tag per line
  • domain can be parsed before entering the loop in line 125 (also you can use tldrextract or a regexp to parse the domain)
  • Same behaviour should be applied at line 196

Thanks for your quick response. If you feel I am too demanding please tell me. I Like your patch (I also like other ones like %YEAR%!!!)!

@nrathaus
Copy link
Author

I did a one tag replacer because EXT had a similar design, to support multiple tags - we would need to rewrite the whole function used - I am ok with doing that

@nrathaus
Copy link
Author

Regarding tldrextract, I didn't want to introduce another library - we don't actually need this in my opinion, just dot breaking is sufficient for this dirsearch improvement

I added additional code, didn't get a chance to test it yet - will try to do it later

@shelld3v
Copy link
Collaborator

shelld3v commented Aug 2, 2024

Another problem I found in this PR: Assuming you run on 2 targets: abc.com and xyz.net, your code will basically create abc.txt and xyz.txt from %DOMAIN%.txt apply on BOTH targets, even though I only want to run abc.txt on abc.com and xyz.txt on xyz.net

@maurosoria
Copy link
Owner

Hello @nrathaus, how you doing?

Do you mind contacting us ? We really want to introduce this PR with @shelld3v !

@nrathaus
Copy link
Author

Not sure how to do that

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.

3 participants