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

Adding a new option to show/hide WHOIS protected data in response #96

Merged
merged 7 commits into from
Feb 6, 2023

Conversation

elshahat
Copy link
Contributor

@elshahat elshahat commented Feb 4, 2023

I have added a new option to the parseDomainWhois function to allow returning the protected whois data based on the true/false value for the newly added option: ignorePrivacy

The default value is true

@elshahat elshahat changed the title Adding a new option to show/hide WHOIS protected data in reponse Adding a new option to show/hide WHOIS protected data in response Feb 4, 2023
@AndreiIgna
Copy link
Member

hey @elshahat thanks for adding this option.
I'll check and test it in the next few days, then include it in the next release

@elshahat
Copy link
Contributor Author

elshahat commented Feb 6, 2023

Hi @AndreiIgna I have included a quick update in the parser file.
In the response body there is a key/value inside the object for the "Last update Last update of WHOIS database" like this:
'>>> Last update of WHOIS database': '2023-02-06T07:00:00Z <<<',

After my update it will be:
'Last update of WHOIS database': '2023-02-06T07:00:00Z',

@AndreiIgna
Copy link
Member

hey @elshahat I just tested both updates, and the first one for ignorePrivacy looks good and ready to merge.

The second one I don't think is ready yet though. I've tested it on a few domains and it has some unintended effects as well.

Before:
Screenshot 2023-02-06 at 14 18 59

After:
Screenshot 2023-02-06 at 14 18 25

There are many WHOIS servers that include html or < > characters in response, and this global replace would modify data that shouldn't be modified.

For the moment, can you revert this commit so we can merge & release the ignorePrivacy option? This change can be done in another Pull Request and with some more specific testing for it

@elshahat
Copy link
Contributor Author

elshahat commented Feb 6, 2023

Hi @AndreiIgna,
Thanks for your response, I have reverted my last commit and will check this change again and try to find a better solution

@AndreiIgna AndreiIgna merged commit d5942b1 into LayeredStudio:master Feb 6, 2023
@AndreiIgna
Copy link
Member

AndreiIgna commented Feb 6, 2023

Thanks, this is now included in release v1.16

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.

None yet

2 participants