Reserve erc-normalize-port for equality comparisons

* etc/ERC-NEWS: Add entry explaining changes to entry point 'erc-tls'
and library functions `erc-normalize-port' and `erc-compute-port'.
* lisp/erc/erc.el (erc-normalize-port): Map "ircu" to 6665 instead of
6667, and add related IANA service 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 machinery
from ever seeing a numeric string, like "6667".
(erc-select-read-args): Since `erc-compute-port' no longer relies on
`erc-normalize-port', ensure its input is a number.  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 keyword argument from Lisp code.
(erc-determine-parameters): Use `erc-compute-port' for initializing
`erc-session-port'.
(erc-compute-port): Don't pass the result through `erc-normalize-port',
which can convert it to an unintuitive form.
(erc--url-default-connect-function): Use `erc-compute-port' instead of
`erc-normalize-port'.
(erc-handle-irc-url): Use `erc-port-equal' for comparison.
* test/lisp/erc/erc-scenarios-auth-source.el
(erc-scenarios-common--auth-source): Allow tests to convey the automatic
port number to `erc-open' via `erc-port'.
(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.
(Bug#74516)

(cherry picked from commit 3bf4ea9543fedb7d3af42d37c853a6f29c681523)

diff --git a/etc/ERC-NEWS b/etc/ERC-NEWS
index 3970f67d725eddf5ca754e51c122b84de3dc8608..f3c8645f02d42949ee04f27162487c59fdff40c9 100644 (file)
--- a/etc/ERC-NEWS
+++ b/etc/ERC-NEWS
@@ -62,6 +62,32 @@ 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' once again considers option 'erc-port'.
+In its zeal to enforce a preference for TLS connections, ERC 5.5 went a
+bit far in disregarding the useful user option 'erc-port'.  When called
+from Lisp code without a ':port' keyword, 'erc-tls' once again respects
+the option.
+
+** 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.
+Moreover, ERC 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.
+
 \f
 * Changes in ERC 5.6
 

diff --git a/lisp/erc/erc.el b/lisp/erc/erc.el
index 5afa8c875b12a8e1bbc6694c040ff236ddd845d8..22b44c79b255b8e26de36de516ef59e1db3e21d8 100644 (file)
--- a/lisp/erc/erc.el
+++ b/lisp/erc/erc.el
@@ -1987,13 +1987,12 @@ the existing buffers will be reused."
                         "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.  Return nil in
+pathological cases."
   ;; These were updated somewhat in 2022 to reflect modern standards
   ;; and practices.  See also:
   ;;
@@ -2001,7 +2000,7 @@ symbol, it may have these values:
   ;; 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 +2010,19 @@ symbol, it may have these values:
         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 +2669,7 @@ side effect of setting the current buffer to the one it returns.  Use
 
     (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 +2773,8 @@ properties needed by entry-point commands, like `erc-tls'."
          (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 +2796,10 @@ properties needed by entry-point commands, like `erc-tls'."
                                (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 +2895,8 @@ and defers to `erc-compute-port', `erc-compute-user', and
 
 ;;;###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
@@ -8838,7 +8843,7 @@ Sets the buffer local variables:
 - `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))
@@ -8911,8 +8916,12 @@ non-nil value is found.
 
 - 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
 
@@ -9878,9 +9887,8 @@ by `erc' and `erc-tls'."
                   (or (eql 6697 (plist-get plist :port))
                       (yes-or-no-p "Connect using TLS? "))))
          (erc-server (plist-get plist :server))
-         (erc-port (or (plist-get plist :port)
-                       (and ircsp (erc-normalize-port 'ircs-u))
-                       erc-port))
+         (erc-default-port (if ircsp erc-default-port-tls erc-default-port))
+         (erc-port (erc-compute-port (plist-get plist :port)))
          (erc-nick (or (plist-get plist :nick) erc-nick))
          (erc-password (plist-get plist :password))
          (args (erc-select-read-args)))
@@ -9912,9 +9920,9 @@ Customize `erc-url-connect-function' to override this."
                            (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 f0a7a4cbaca34bf0622fdb900ef9d9ec9b76a784..7e1e7c2f3ab52c2b87172a0242b3c1990fd2fc51 100644 (file)
--- a/test/lisp/erc/erc-scenarios-auth-source.el
+++ b/test/lisp/erc/erc-scenarios-auth-source.el
@@ -44,15 +44,19 @@
                                    (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 @@
 
 (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 7a4df04794e16392264c4aa5067e1389cb4ea32e..596b1fd13caa80a8bcbe93449a1ade763bf4d698 100644 (file)
--- a/test/lisp/erc/erc-tests.el
+++ b/test/lisp/erc/erc-tests.el
@@ -3057,6 +3057,22 @@
     (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"