client icon indicating copy to clipboard operation
client copied to clipboard

Is the isNeighborUri same as isUri in iota.js?

Open peter279k opened this issue 7 years ago • 3 comments

As title. Considering the isUri function in iota.js. The valid examples are as follows:

*   udp://[2001:db8:a0b:12f0::1]:14265
*   udp://[2001:db8:a0b:12f0::1]
*   udp://8.8.8.8:14265
*   udp://domain.com
*   udp://domain2.com:14265

The isNeighborUri method in IOTA\Util\ValidatorUtil class. All of the valid uris are the neighbor uri.

It seems that they're different. For example, the isUri only supports the tcp or udp.

I want to know isNeighborUri is implemented correctly?

peter279k avatar Aug 24 '18 17:08 peter279k

Well, we use filter_var($neighborUri, FILTER_VALIDATE_URL) and this will also, for instance, allow http://www.iota.org:12345 - so it's not the same.

The isUri implementation is much more detailed and restrictive.

Im not sure if we really need to check this in detail like the JS lib does, as the IRI API commands addNeighbors and removeNeighbors should return an error if the URI is really wrong and we could handles that.

I don't want to shift the responsibility of checking everything in detail prior sending it to the API to the client, this needs to be implemented in the server and the server should react accordingly and send an error that then can be handled by the client.

So from my point of view implementing this check in detail will add technical debt to the project, especially because of the monstrous regex they are using.

If we ever get to the point that we will create our own IRI implementation in PHP it will be useful, but I doubt this will happen (even though it would be really interesting!)

Techworker avatar Aug 24 '18 20:08 Techworker

@Techworker, thank you for your explanation. I think I agree with you because we don't validate the url strictly. Just check whether the uri is the format.

I have little question. Using the FILTER_VALIDATE_URL to validate uri and the uri can accept the path, query and fragment.

I think we can check that and validate the uri only has the authority.

What do you think?

peter279k avatar Aug 25 '18 00:08 peter279k

I don't see the immediate need but if you want to add this functionality, go for it. Won't hurt anyone to have this check :-)

Techworker avatar Aug 25 '18 11:08 Techworker