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 extension method to retrieve the collection of rules from a struc… #67

Merged
merged 5 commits into from
Mar 24, 2024

Conversation

merijndejonge
Copy link

This pull requests adds an extension method and corresponding unit tests for retrieving the collection of TldRules from a structure. This is needed for more advanced rule providers, such as rule combinators that combine the rules from multiple rule providers. Since the parsed rules are pretty-much hidden in the structure, this extension method is needed to provide access to the rules.

@tinohager
Copy link
Member

somehow I think there are too many changes in the pull request

@merijndejonge
Copy link
Author

This pull requests only adds a single extension method and corresponding unit tests. Please reconsider

@merijndejonge
Copy link
Author

@tinohager any problems with this pull request that adds an extension method for retrieving the collection of TldRules from a structure?

@tinohager
Copy link
Member

I would like to know the reason for the extension. Why are the domains that you still need not listed in the official list?

Why should it be integrated into the main library and not be an add-on package?

@merijndejonge
Copy link
Author

  • Implement the extension method in an add-on package is no big deal. Actually, that is how I'm currently using it. But I propose to donate it so that more people can use it and such that everything is contained in one code base that needs to be maintained.
  • The TldRules in a DomainDataStructure are hidden and cannot be accessed easily. There are AddTule/AddRules methods but no corresponding GetRule/GetRules methods. I don't see what the reason is for the asymmetric design. This hampers e.g., inspection of the rules once they are consumed by a DomainDataStructure. The proposed extension method makes your design more symmetric (which is another reason to put in your code base :-) )
  • I use your library for the implementation of a password manager. The passwords follow public suffix rules. However, for this application, the public suffix list is far from complete. For instance, an organisation like auth0.com uses different identity systems for its subdomains (e.g., customera.auth0.com and customerb.auth0.com). With the information from just the public suffix list, we cannot distinguish between passwords from different customers. Therefore we add auth0.com as an extra entry to solve this. Your library is extremely useful for this, but just a bit too limited. With a few small modifications the design improves making room for use cases like mine.

@tinohager tinohager merged commit b20ee2b into nager:main Mar 24, 2024
1 check passed
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