-
Notifications
You must be signed in to change notification settings - Fork 44.6k
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
Allow port numbers to be used in local host names. e.g. localhost:8888. #5318
Conversation
allow ports to be used in host names. e.g. localhost:8888. (Note: code generated by ChatGPT.)
✅ Deploy Preview for auto-gpt-docs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
# List of local hostnames/IPs without considering ports | ||
local_domains = [ | ||
"localhost", | ||
"2130706433", # IP representation of 127.0.0.1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TIL :)
"file:///", | ||
"file://localhost/", | ||
"file://localhost", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On what systems would this work?
Also kudo's for indicating that the code was generated! |
- Modify the check_local_file_access function to only check for local file prefixes and return True if the URL starts with any of them. - Remove the section of code that parsed the URL and checked if the hostname was in a list of local domains. This change fixes the URL validation in the validators.py file. Previously, only local domains were allowed. After the change, non-local domains are allowed again. (Note: this change was made in response to PR #5318 where the validation was modified to allow more local domains, but also accidentally to block any non-local domain.
allow ports to be used in host names. e.g. localhost:8888. Note: code copied from pull request 5318, Significant-Gravitas#5318. I did not test this.
Allow port numbers to be used in local host names. e.g. localhost:8888.
Background
There are scenarios where local development or specific server configurations require non-standard ports to be used in conjunction with hostnames such as localhost. The current implementation of check_local_file_access does not allow for URLs with such ports, making it inconvenient or impossible for developers and users to utilize the software in some local environments. This pull request updates the function to be more accommodating to such URLs, enhancing the tool's flexibility.
Changes 🏗️
Modified the check_local_file_access function in url_validator.py to accommodate URLs with ports in host names.
Removed redundant entries in the local_prefixes list.
PR Quality Scorecard ✨
+2 pts
+5 pts
+5 pts
+5 pts
-4 pts
+4 pts
+5 pts
-5 pts
agbenchmark
to verify that these changes do not regress performance?+10 pts