marceldev89 / BattleNET

BattlEye Protocol Library and Client
GNU Lesser General Public License v3.0
76 stars 44 forks source link

DNS Support #19

Closed UncleDave closed 11 years ago

UncleDave commented 11 years ago

Could we possibly get DNS support in the next version? It's really simple to add something like:

System.Net.IPAddress targetHost =  System.Net.Dns.GetHostAddresses(ipOrHostString)[0];

This returns the first address for the given host OR address, so if you give it an IP it'll return the same IP, which obviously means you don't have to figure out if the input is a hostname or an IP.

I realise this can be done just as easily by implementation but it seems to me to be a feature that the library would benefit from.

marceldev89 commented 11 years ago

This is planned. Should be in the next update.

marceldev89 commented 11 years ago

Alright, I'm working on this but how does this benefit the library exactly? It seems to me this makes it more complicated than it has to be.

client > verify ip or host > send to library client > send to library > verify ip or host (by library) > send message back to client > check messages if host was invalid (by client)

UncleDave commented 11 years ago

I don't see how this really changes anything.

Instead of

IPAddress ipAddress = IPAddress.Parse(loginCredentials.Host);

You have:

IPAddress ipAddress =  Dns.GetHostAddresses(loginCredentials.Host)[0];

And everything works in the exact same way, does it not? At least this was my experience when I had written my own connector.

marceldev89 commented 11 years ago

Yes but what if it's not a valid hostname or ip? The lib would then have to explain this to the app that implements the library.

UncleDave commented 11 years ago

Well what if it's not a valid ip in your current system? Maybe i'm missing something but I don't see how the way invalid entries are handled changes here. You're still ending up with an IPAddress in the exact same place, provided loginCredentials.Host is valid, and if it's not you have the same issue as if it's not valid when you attempt to parse it.

marceldev89 commented 11 years ago

You're right, with or without it, it would still fail miserably. :)

marceldev89 commented 11 years ago

Here you go: a23fbb9f451b02a3d3558e29cd389afe559b6054.

I still think this is somewhat redundant because the implementing application still has to check whether the input was valid anyway.

old: client > get host info > resolve hostname or ip / validate > connect

vs

new: client > get host info > resolve hostname or ip / validate > connect > resolve hostname or ip

Not entirely sure if it will end up in 1.3 but it's there for now.

UncleDave commented 11 years ago

I just find it rather dirty to turn a string into an IP then back to a string to be sent to the library.

At present in my application the only validation I do on submit is making sure the text box isn't empty, if the hostname/ip turns out to not be valid then your library sends a socket exception message, and I'm just fine with that.

Or could you not just wrap it in a try and return Error if it can't resolve?

DomiStyle commented 11 years ago
        bool isIP(String ip)
        {
            string[] parts = ip.Split('.');
            if (parts.Length < 4)
            {
                return false;
            }
            else
            {
                foreach (string part in parts)
                {
                    byte checkPart = 0;
                    if (!byte.TryParse(part, out checkPart))
                    {
                        return false;
                    }
                }
                return true;
            }
        }

Works quite fine to detect if a String is an IP address.

marceldev89 commented 11 years ago

@DomiStyle

bool isIP(string ip)
{
    IPAddress value;
    (IPAddress.TryParse(ip, out value)) ? return true : return false;
}

Anyway, in my opinion, the library should only have to work with IP addresses and it's the job of the implementing application to make sure that happens. :D

marceldev89 commented 11 years ago

Perhaps something can be done with the logincredentials class somehow.

UncleDave commented 11 years ago

To be honest I'm still not exactly sure what the issue with adding this is. The events such as "Connection Failed" and "Socket Error" are already in place to use if an IP/Host fails to resolve/parse.

marceldev89 commented 11 years ago

Alright, let's take PHP as an example. This library is PHP, the website is you (the implementation). You've setup a webpage that takes certain values and writes that to a database.

The end user supplies the values, the webpage checks whether the values are what they are supposed to be, and PHP doesn't have to care about it.

I know BattleNET is far from what PHP but hopefully it explains my point of view.

marceldev89 commented 11 years ago

Oh and, BattlEyeLoginCredentials.Host will be changed to IPAddress instead of string. Removing the need for further conversion.

UncleDave commented 11 years ago

That also works fine for me :smile: