From 420463c7f89a708ab0cc80e5bb2f6715fc48a03c Mon Sep 17 00:00:00 2001 From: "F. Jason Park" Date: Mon, 25 Nov 2024 04:21:36 -0800 Subject: [PATCH 2/2] [5.6.1] Only normalize ports for equality comparisons in ERC * etc/ERC-NEWS: Add entry explaining changes to entry point 'erc-tls' and library functions, like `erc-normalize-port' and `erc-compute-port'. * lisp/erc/erc.el (erc-normalize-port): Map "ircu" to 6665 instead of 6667. Add other IANA mappings. Return 0 for unknown nonempty strings. (erc-open): Pass `erc-session-port' through `erc-string-to-port' before handing it to `erc-server-connect'. This prevents the network-connection machinery from seeing a numeric string, like "6667". (erc-select-read-args): Use `erc-port-equal' instead of `eql'. (erc-tls): Respect a configured non-nil `erc-port' option when the user does not provide a :port argument. (erc-determine-parameters): Use `erc-compute-port' for `erc-session-port'. (erc-compute-port): Don't pass the result through `erc-normalize-port', which can convert it to an unintuitive form. (erc-handle-irc-url): Use `erc-port-equal' for comparison. (Bug#74516) * test/lisp/erc/erc-scenarios-auth-source.el (erc-scenarios-common--auth-source) (erc-scenarios-base-auth-source-server--dialed): Use `erc-port' option instead of passing a :port parameter to entry-point command. * test/lisp/erc/erc-tests.el (erc-normalize-port): New test. --- etc/ERC-NEWS | 25 +++++++++++ lisp/erc/erc.el | 52 +++++++++++++--------- test/lisp/erc/erc-scenarios-auth-source.el | 29 +++++++----- test/lisp/erc/erc-tests.el | 16 +++++++ 4 files changed, 88 insertions(+), 34 deletions(-) diff --git a/etc/ERC-NEWS b/etc/ERC-NEWS index 3970f67d725..1c7c278338f 100644 --- a/etc/ERC-NEWS +++ b/etc/ERC-NEWS @@ -62,6 +62,31 @@ of concerns and the newer module's "experimental" status, the migration was deemed worth any potential disruption, despite this being a point release. ERC appreciates your understanding in this matter. +** Entry-point command 'erc-tls' now considers option 'erc-port'. +When called in Lisp code, this command now respects a non-nil 'erc-port' +if the ':port' keyword is absent. This also means users with this +option set to a non-TLS port should make sure to specify a ':port' from +now on. + +** Changes in the library API. + +*** Function 'erc-normalize-port' may return 0 instead of nil. +When given a nonempty, non-numeric string, this function now returns 0. +ERC now officially requests that users not use its output for anything +but comparing port equality, which was always its intended purpose. + +*** Function 'erc-compute-port' no longer uses 'erc-normalize-port'. +An uninformed change in ERC 5.5 led to 'erc-compute-port' filtering its +result through 'erc-normalize-port', which brought unwelcome type +coercion and possible null return values. This defied its purpose of +ensuring a usable port. Users reliant on the aberrant 5.5 behavior +should wrap its return value in 'erc-normalize-port'. + +*** Local variable 'erc-session-port' may be a string. +Although this has always been the case, string values are now more +likely to be seen because ERC no longer coerces service names to port +numbers. + * Changes in ERC 5.6 diff --git a/lisp/erc/erc.el b/lisp/erc/erc.el index 60d84b2b20c..46f096142d6 100644 --- a/lisp/erc/erc.el +++ b/lisp/erc/erc.el @@ -1987,13 +1987,11 @@ erc-reuse-buffers "old behavior when t now permanent" "29.1") (defun erc-normalize-port (port) - "Normalize the port specification PORT to integer form. -PORT may be an integer, a string or a symbol. If it is a string or a -symbol, it may have these values: -* irc -> 194 -* ircs -> 994 -* ircd -> 6667 -* ircd-dalnet -> 7000" + "Normalize known PORT specifications to an integer. +Expect PORT to be an integer, a string, or a symbol to coerce into a +standardized form for the express purpose of equality comparisons. If +PORT is an IANA recognized service, return its numeric mapping. Do the +same for a few traditional but nonstandard names." ;; These were updated somewhat in 2022 to reflect modern standards ;; and practices. See also: ;; @@ -2001,7 +1999,7 @@ erc-normalize-port ;; https://www.iana.org/assignments/service-names-port-numbers (cond ((symbolp port) - (erc-normalize-port (symbol-name port))) + (and port (erc-normalize-port (symbol-name port)))) ((stringp port) (let ((port-nr (string-to-number port))) (cond @@ -2011,14 +2009,19 @@ erc-normalize-port 194) ((string-equal port "ircs") 994) - ((string-equal port "ircu") 6667) ; 6665-6669 + ((string-equal port "ircu") 6665) + ((string-equal port "ircu-2") 6666) + ((string-equal port "ircu-3") 6667) + ((string-equal port "ircu-4") 6668) + ((string-equal port "ircu-5") 6669) ((string-equal port "ircd") ; nonstandard (irc-serv is 529) 6667) ((string-equal port "ircs-u") 6697) ((string-equal port "ircd-dalnet") 7000) + ((string-empty-p port) nil) (t - nil)))) + 0)))) ((numberp port) port) (t @@ -2665,7 +2668,7 @@ erc-open (if connect (erc-server-connect erc-session-server - erc-session-port + (erc-string-to-port erc-session-port) buffer erc-session-client-certificate) (erc-update-mode-line)) @@ -2769,8 +2772,8 @@ erc-select-read-args (port (or (url-portspec url) (erc-compute-port (let ((d (erc-compute-port sp))) ; may be a string - (read-string (format-prompt "Port" d) - nil nil d))))) + (erc-string-to-port + (read-string (format-prompt "Port" d) nil nil d)))))) ;; Trust the user not to connect twice accidentally. We ;; can't use `erc-already-logged-in' to check for an existing ;; connection without modifying it to consider USER and PASS. @@ -2792,10 +2795,10 @@ erc-select-read-args (format-prompt "Server password" p) "Server password (optional): "))) (if erc-prompt-for-password (read-passwd m nil p) p))) - (opener (and (or sp (eql port erc-default-port-tls) + (opener (and (or sp (erc-port-equal port erc-default-port-tls) (and (equal server erc-default-server) (not (string-prefix-p "irc://" input)) - (eql port erc-default-port) + (erc-port-equal port erc-default-port) (y-or-n-p "Connect using TLS instead? ") (setq port erc-default-port-tls))) #'erc-open-tls-stream)) @@ -2891,7 +2894,8 @@ 'erc-ssl ;;;###autoload (cl-defun erc-tls (&key (server (erc-compute-server)) - (port (erc-compute-port 'ircs-u)) + (port (let ((erc-default-port erc-default-port-tls)) + (erc-compute-port))) (nick (erc-compute-nick)) (user (erc-compute-user)) password @@ -8841,7 +8845,7 @@ erc-determine-parameters - `erc-server-current-nick'" (setq erc-session-connector erc-server-connect-function erc-session-server (erc-compute-server server) - erc-session-port (or port erc-default-port) + erc-session-port (erc-compute-port port) erc-session-user-full-name (erc-compute-full-name name) erc-session-username (erc-compute-user user) erc-session-password (erc--compute-server-password passwd nick)) @@ -8914,8 +8918,12 @@ erc-compute-port - PORT (the argument passed to this function) - The `erc-port' option -- The `erc-default-port' variable" - (erc-normalize-port (or port erc-port erc-default-port))) +- The `erc-default-port' variable + +Note that between ERC 5.5 and 5.6.1, this function filtered its result +through `erc-normalize-port', which introduced regrettable surprises, +such as unwelcome, possibly null, type conversions." + (or (and port (not (equal "" port)) port) erc-port erc-default-port)) ;; time routines @@ -9909,9 +9917,9 @@ erc-handle-irc-url (and (string-equal erc-session-server host) ;; Ports only matter when dialed hosts ;; match and we have sufficient info. - (or (not port) - (= (erc-normalize-port erc-session-port) - port))))))))) + (or (null port) + (erc-port-equal erc-session-port + port))))))))) key deferred) (unless server-buffer (setq deferred t diff --git a/test/lisp/erc/erc-scenarios-auth-source.el b/test/lisp/erc/erc-scenarios-auth-source.el index f0a7a4cbaca..7e1e7c2f3ab 100644 --- a/test/lisp/erc/erc-scenarios-auth-source.el +++ b/test/lisp/erc/erc-scenarios-auth-source.el @@ -44,15 +44,19 @@ erc-scenarios-common--auth-source (string-join ents "\n"))) (auth-sources (list netrc-file)) (auth-source-do-cache nil) + (erc-port (and (eq erc-port 'test) (number-to-string port))) (erc-scenarios-common-extra-teardown (lambda () - (delete-file netrc-file)))) + (delete-file netrc-file))) + ;; With a `cl-defun', a keyword's presence prevents the default + ;; init form from being evaluated, even if its value is nil. + (args `( :server "127.0.0.1" + ,@(and (null erc-port) (list :port port)) + :nick "tester" + :full-name "tester" + :id ,id))) (ert-info ("Connect") - (with-current-buffer (erc :server "127.0.0.1" - :port port - :nick "tester" - :full-name "tester" - :id id) + (with-current-buffer (apply #'erc args) (should (string= (buffer-name) (if id (symbol-name id) (format "127.0.0.1:%d" port)))) @@ -60,12 +64,13 @@ erc-scenarios-common--auth-source (ert-deftest erc-scenarios-base-auth-source-server--dialed () :tags '(:expensive-test) - (erc-scenarios-common--auth-source - nil 'foonet - "machine GNU.chat port %d user tester password fake" - "machine FooNet port %d user tester password fake" - "machine 127.0.0.1 port %d user tester password changeme" - "machine 127.0.0.1 port %d user imposter password fake")) + (let ((erc-port 'test)) + (erc-scenarios-common--auth-source + nil 'foonet + "machine GNU.chat port %d user tester password fake" + "machine FooNet port %d user tester password fake" + "machine 127.0.0.1 port \"%s\" user tester password changeme" ; correct + "machine 127.0.0.1 port %d user imposter password fake"))) (ert-deftest erc-scenarios-base-auth-source-server--netid () :tags '(:expensive-test) diff --git a/test/lisp/erc/erc-tests.el b/test/lisp/erc/erc-tests.el index 4c5521721f0..c6936ba8dc0 100644 --- a/test/lisp/erc/erc-tests.el +++ b/test/lisp/erc/erc-tests.el @@ -2978,6 +2978,22 @@ erc--route-insertion (should-not (buffer-live-p spam-buffer)) (kill-buffer chan-buffer))) +(ert-deftest erc-normalize-port () + ;; The empty string, nil, and unsupported types become nil. + (should-not (erc-normalize-port "")) + (should-not (erc-normalize-port nil)) + (should-not (erc-normalize-port (current-buffer))) + + ;; Unrecognized names are coerced to 0. + (should (equal 0 (erc-normalize-port "fake"))) + + ;; Numbers pass through, but numeric strings are coerced. + (should (equal 6667 (erc-normalize-port 6667))) + (should (equal 6697 (erc-normalize-port "6697"))) + + ;; Strange IANA mappings recognized. + (should (equal 6665 (erc-normalize-port "ircu")))) + (defvar erc-tests--ipv6-examples '("1:2:3:4:5:6:7:8" "::ffff:10.0.0.1" "::ffff:1.2.3.4" "::ffff:0.0.0.0" -- 2.47.0