[http] Replace deprecated URL constructor #19519
Conversation
Signed-off-by: Leo Siepel <leosiepel@gmail.com>
There was a problem hiding this comment.
Pull Request Overview
This PR replaces the deprecated java.net.URL constructor with a custom URI parsing implementation to modernize the HTTP binding's URL handling approach.
Key Changes:
- Removed dependency on deprecated
URLclass constructor - Implemented custom regex-based URL parsing with
parse()method - Maintained existing functionality for IDN (Internationalized Domain Names) conversion and special character handling
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| return URI.create(uri.toASCIIString().replace("+", "%2B").replace("%25%25", "%")); | ||
| } | ||
|
|
||
| private static final Pattern URL_PATTERN = Pattern.compile("^(?:(?<scheme>[a-zA-Z][a-zA-Z0-9+.-]*):)?//?" |
There was a problem hiding this comment.
The regex pattern makes the '//' after the scheme optional with '//?', which incorrectly allows malformed URLs like 'http:example.com'. RFC 3986 requires '//' for authority-based URIs. The pattern should use '//(' to make the entire authority section optional, or '//' if authority is required.
| private static final Pattern URL_PATTERN = Pattern.compile("^(?:(?<scheme>[a-zA-Z][a-zA-Z0-9+.-]*):)?//?" | |
| private static final Pattern URL_PATTERN = Pattern.compile("^(?:(?<scheme>[a-zA-Z][a-zA-Z0-9+.-]*):)?//" |
Copilot uses AI. Check for mistakes.
| } | ||
|
|
||
| private static final Pattern URL_PATTERN = Pattern.compile("^(?:(?<scheme>[a-zA-Z][a-zA-Z0-9+.-]*):)?//?" | ||
| + "(?:(?<userinfo>[^@/?#]*)@)?" + "(?<host>[^:/?#]*)(?::(?<port>\\d+))?" + "(?<path>/[^?#]*)?" |
There was a problem hiding this comment.
The path pattern requires a leading slash with '/[^?#]', but paths can be relative or empty. This will fail to parse valid URIs like 'http://example.com' (empty path) or 'http://example.com?query' (no path). Change to '(?[^?#])?' to allow any path including empty.
| + "(?:(?<userinfo>[^@/?#]*)@)?" + "(?<host>[^:/?#]*)(?::(?<port>\\d+))?" + "(?<path>/[^?#]*)?" | |
| + "(?:(?<userinfo>[^@/?#]*)@)?" + "(?<host>[^:/?#]*)(?::(?<port>\\d+))?" + "(?<path>[^?#]*)?" |
Copilot uses AI. Check for mistakes.
bundles/org.openhab.binding.http/src/main/java/org/openhab/binding/http/internal/Util.java
Show resolved
Hide resolved
bundles/org.openhab.binding.http/src/main/java/org/openhab/binding/http/internal/Util.java
Outdated
Show resolved
Hide resolved
…ding/http/internal/Util.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: lsiepel <leosiepel@gmail.com>
…ding/http/internal/Util.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: lsiepel <leosiepel@gmail.com>
|
Regarding the co-pilot suggestions about the regex. Unless some one is absolutely sure, i like to stick to the current implementation to prevent regression. |
1 participant
As a follow-up to #19159 specific for the http binding