From 96a4de741663672e928fd30af6c93b335b346691 Mon Sep 17 00:00:00 2001 From: "F. Jason Park" Date: Tue, 29 Nov 2022 00:18:42 -0800 Subject: [PATCH 0/4] *** NOT A PATCH *** *** BLURB HERE *** F. Jason Park (4): ; Don't hard code server ports in SOCKS tests [30.0.50] Improve SOCKS error handling and add support for 4a [WIP/30.0.50] Simplify network-stream openers in socks.el [POC/30.0.50] Integrate the socks and url libraries lisp/net/socks.el | 142 +++++++++++++++++++++++++++++------ lisp/url/url-gw.el | 8 +- lisp/url/url-http.el | 16 ++-- lisp/url/url-proxy.el | 18 ++++- lisp/url/url-vars.el | 13 +++- test/lisp/net/socks-tests.el | 39 +++++++--- 6 files changed, 186 insertions(+), 50 deletions(-) Interdiff: diff --git a/lisp/net/socks.el b/lisp/net/socks.el index ac732b228b..65436ed047 100644 --- a/lisp/net/socks.el +++ b/lisp/net/socks.el @@ -325,20 +325,20 @@ socks-filter (process-put proc 'socks-response string)))))) ((= state socks-state-connected))))) +;; FIXME this is a terrible idea. +;; It is not even compatible with the argument spec of open-network-stream +;; in 24.1. + (defvar socks-override-functions nil "If non-nil, overwrite `open-network-stream' function with SOCKSified version.") -;; Libraries typically offer a "stream opener" option, such as ERC's -;; `erc-server-connect-function'. These provide a level of -;; flexibility tantamount to what this variable formerly offered. -(make-obsolete-variable - 'socks-override-functions - "see `socks-open-network-stream' and `socks-connect-function'." "29.1") +(when socks-override-functions + (advice-add 'open-network-stream :around #'socks--open-network-stream)) (defcustom socks-connect-function 'open-network-stream "Function to open a network connection to a SOCKS provider. Called with arguments suitable for `open-network-stream'." - :version "29.1" + :version "30.1" :type '(choice (function-item :value open-network-stream) (function :tag "User-provided function"))) @@ -348,14 +348,11 @@ socks-open-connection form of `socks-server' and STREAM-PARAMS to be keyword params accepted by `open-network-stream'." (interactive) - (unless (plist-member stream-params :coding) - (setf (plist-get stream-params :coding) '(binary . binary))) (save-excursion (let ((proc - (with-suppressed-warnings ((obsolete socks-override-functions)) - (let ((socks-override-functions nil)) - (apply socks-connect-function (nth 0 server-info) nil - (nth 1 server-info) (nth 2 server-info) stream-params)))) + (let ((socks-override-functions nil)) + (apply socks-connect-function (nth 0 server-info) nil + (nth 1 server-info) (nth 2 server-info) stream-params))) (authtype nil) version) @@ -474,11 +471,11 @@ socks-send-command nil ; Sweet sweet success! (delete-process proc) (error "SOCKS: %s" - (let ((no (or (process-get proc 'socks-reply) 99))) - (or (if (eq version 5) ; 99 - 90 >= length(errors) - (nth no socks-errors) - (nth (- no 90) socks--errors-4)) - "Unknown error")))) + (let ((err (process-get proc 'socks-reply))) + (if (eql version 5) + (nth (or err 1) socks-errors) + (nth (- (if (and err (<= 90 err 93)) err 91) 90) + socks--errors-4))))) proc)) @@ -539,16 +536,15 @@ socks-find-services-entry (gethash (downcase service) (if udp socks-udp-services socks-tcp-services))) -(defcustom socks-open-network-stream-fallback nil - "Whether `socks-open-network-stream' should fall back to non-SOCKS." - :version "29.1" - :type 'boolean) +(defun socks--open-network-stream (orig-fun name buffer host service &rest params) + (let ((socks-override-functions orig-fun)) + (apply #'socks-open-network-stream name buffer host service params))) (defcustom socks-proxied-tls-services '(443 6697) "Ports whose connections should use TLS. Note that the system resolver may be consulted to look up host names for checking domain validation certs." - :version "29.1" + :version "30.1" :type '(repeat number)) (declare-function gnutls-negotiate "gnutls" (&rest rest)) @@ -568,9 +564,12 @@ socks-open-network-stream though `socks-connect-function' has the final say). For TLS with proxied connections, see the option `socks-proxied-tls-services'. -Before connecting, check the host against `socks-noproxy', and on -rejection either signal an error or fall back to non-SOCKS, -depending on the value of `socks-open-network-stream-fallback'. +Before connecting, check the host against `socks-noproxy' and, on +rejection, fall back to non-SOCKS. Similarly, when +`socks-override-functions' is a function, call it directly and +trust that it's not interested in options defined in this +library, such as `socks-server'. + But, before doing anything, check if `url-using-proxy' is bound to a `url' struct object, as defined in `url-parse'. If so, assume it represents the address of the desired SOCKS server @@ -591,13 +590,14 @@ socks-open-network-stream socks-username)) (socks-password (or (and url (url-password url)) socks-password))) - (if-let* ((route (socks-find-route host service)) + (if-let* (((booleanp socks-override-functions)) + (route (socks-find-route host service)) (proc (apply #'socks-open-connection route params))) (let ((port (if (numberp service) service (process-contact proc :service))) (certs (plist-get params :client-certificate))) - (socks--open-network-stream proc buffer host service) + (socks--initiate-command-connect proc buffer host service) (if (and (memq port socks-proxied-tls-services) (gnutls-available-p) (require 'gnutls nil t) @@ -607,16 +607,15 @@ socks-open-network-stream :keylist (and certs (list certs))) (nsm-verify-connection proc host port)) proc)) - ;; Retain legacy behavior and connect anyway without warning - (if socks-open-network-stream-fallback - (with-suppressed-warnings ((obsolete socks-override-functions)) - (let (socks-override-functions) - (apply #'open-network-stream name buffer host service params))) - (error "Connection rejected by `socks-noproxy'"))))) - -(defun socks--open-network-stream (proc buffer host service) + (let ((fn (if (functionp socks-override-functions) + socks-override-functions ; `socks-noproxy' not consulted + #'open-network-stream)) ; `socks-noproxy' is non-nil + socks-override-functions) + (apply fn name buffer host service params))))) + +(defun socks--initiate-command-connect (proc buffer host service) (progn ; preserve indentation level for git blame / code review - (progn ; could rename to something like `socks--initiate-command-connect' + (progn (let* ((version (process-get proc 'socks-server-protocol)) (atype (cond -- 2.38.1