uri: Fix handling of empty ports for uri_parser_rfc3986 #19645
Conversation
TimWolla
force-pushed
the
uri-rfc3986-empty-port
branch
from
258c8f7 to
5c41900
Compare
Yesn't. The validation in the constructor treats the empty port as |
| zend_throw_exception(uri_invalid_uri_exception_ce, "The port is out of range", 0); | ||
| } | ||
| if (has_text_range(&uri.portText) && get_text_range_length(&uri.portText) > 0) { | ||
| if (port_str_to_zend_long_checked(uri.portText.first, get_text_range_length(&uri.portText)) == -1) { |
There was a problem hiding this comment.
nit: why is the separate if? It seems a bit off
There was a problem hiding this comment.
The outer if is “checking if we have a port” and the inner if is “checking if the port is valid”. Two separate concerns, thus two separate if()s.
There was a problem hiding this comment.
I find it strange because we are not interested in the former concern, only the latter is needed. But I don't mind the extra indentation too much to bother with this formatting nuance :)
This is currently leaking due to uriparser/uriparser#250, thus leaving on draft for now.