momu commented 2 weeks ago

Please verify that this bug has NOT been raised before.

[x] I checked and didn't find a similar issue

Describe the bug*

When adding an external link when editing a stock item, only a fully qualified domain name is permitted in the URL, e.g. https://foo.bar.

When entering a URL which consists only of a hostname e.g. https://foo, an error is raised (Enter a valid URL):

The validation should be enhanced that hostname only is also allowed .

Steps to Reproduce

Go to a stock item
Edit stock item
try to enter https://foo as external link

Expected behaviour

When entering https://foo as external link it should be accepted an the stock item should be saved.

Deployment Method

[ ] Docker
[ ] Package
[x] Bare metal
[ ] Other - added info in Steps to Reproduce

Version Information

Version Information:

InvenTree-Version: 0.16.2 Django Version: 4.2.15 Commit Hash: 065aca4 Commit Date: 2024-09-10

Database: postgresql Debug-Mode: True Deployed using Docker: False Platform: Linux-5.15.0-122-generic-x86_64-with-glibc2.35 Installer: GIT

Active plugins: False

Please verify if you can reproduce this bug on the demo site.

[x] I can reproduce this bug on the demo site.

Relevant log output

No response

matmair commented 2 weeks ago

Resolving adresses like that serverside poses risks of exposing internal network information, I do not see this as a bug.

SchrodingersGat commented 2 weeks ago

@matmair I do not think this requires resolving anything on the server side - this is just validation of a text field, right?

momu commented 2 weeks ago

@SchrodingersGat @matmair

Yes it's just about validation of the user entry. No need of network resolving anything.

As of now the external link feature is not usable in my case. I'm using InvenTree in the intranet and have a GitLab instance also running there. When it comes to stock parts I've used the link feature to link to certain labeled issues.

As our network infrastructure changed, there is now only the hostname, so I can't enter new external links or update the existing ones.

Seems to me like a bug, as RFC2396 states that the hostname can contain the toplabel only:

      host          = hostname | IPv4address
      hostname      = *( domainlabel "." ) toplabel [ "." ]

SchrodingersGat commented 1 week ago

This is a restriction in django itself - https://docs.djangoproject.com/en/4.2/ref/validators/#urlvalidator

The URLValidator class (which we sub-class in our code) prevents non-loopback addresses which do not specify a TLD. e.g.

Valid

Invalid

I'm not sure if there is a clean way for us to code around this, even if we wanted to

matmair commented 1 week ago

We are using the same validator for this specific field as the fields where we eventually call them so changes should be made carefully. I find running a network without any tldr to be a very outlandish edge case and not something that can be expected to be supported.