From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Michael Olson Newsgroups: gmane.emacs.devel Subject: Re: [PATCH] etc-dcc: Fix handling SEND commands with unquoted filename. Date: Wed, 9 May 2012 08:29:30 -0700 Message-ID: References: <20120509175758.6a613706@sacrilege> NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: multipart/alternative; boundary=0015175cf764a3399504bf9c2c49 X-Trace: dough.gmane.org 1336577418 28087 80.91.229.3 (9 May 2012 15:30:18 GMT) X-Complaints-To: usenet@dough.gmane.org NNTP-Posting-Date: Wed, 9 May 2012 15:30:18 +0000 (UTC) Cc: Julien Danjou , emacs-devel@gnu.org To: Mike Kazantsev Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Wed May 09 17:30:15 2012 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1SS8qC-0001IP-8Z for ged-emacs-devel@m.gmane.org; Wed, 09 May 2012 17:30:12 +0200 Original-Received: from localhost ([::1]:46912 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SS8qB-0004rj-IO for ged-emacs-devel@m.gmane.org; Wed, 09 May 2012 11:30:11 -0400 Original-Received: from eggs.gnu.org ([208.118.235.92]:45390) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SS8q4-0004nx-Ac for emacs-devel@gnu.org; Wed, 09 May 2012 11:30:09 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SS8pv-0004AT-Mx for emacs-devel@gnu.org; Wed, 09 May 2012 11:30:03 -0400 Original-Received: from mail-bk0-f41.google.com ([209.85.214.41]:42029) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SS8pv-0004A8-7T for emacs-devel@gnu.org; Wed, 09 May 2012 11:29:55 -0400 Original-Received: by bkcjm19 with SMTP id jm19so491077bkc.0 for ; Wed, 09 May 2012 08:29:52 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=mime-version:sender:x-originating-ip:in-reply-to:references:from :date:x-google-sender-auth:message-id:subject:to:cc:content-type :x-gm-message-state; bh=r1EpVfCe0jWoOIDOXeZvLSno2LqEsJKFiLYbCMKmv4g=; b=G3zVEVcjDyzuEs1Zq31PRW15yJdSedFtSHbyQsa8I5sZROyk0E09BbtDbGuBSsRTeq C2EgO2le8Of44AUKhUzvDTs8URuG/d8bnQMW7tUZapxDU2t+U8ptqkdlPpuUN71OvOOS h9VXnevYaQ/FbIK+/93pzSpBXYsPnASFe9wKNEaL944+3y2atEuU98BzZFYp9xeWIKg+ 0c2VvbvIX7Ly28kA9SGe4yDMsVkOUggtZl/rHtjGJ1FbQ1f28+75HH7F9Q0SB2CbhC9s lg+g2ddW94dM1EtrVr+BGNDzJ2R/j6JWYf7KQwVAXyIWQ23UwwTOX8/8RzXMIcaRAVJ7 nwjw== Original-Received: by 10.204.152.137 with SMTP id g9mr189447bkw.95.1336577391540; Wed, 09 May 2012 08:29:51 -0700 (PDT) Original-Received: by 10.204.231.75 with HTTP; Wed, 9 May 2012 08:29:30 -0700 (PDT) X-Originating-IP: [99.116.250.80] In-Reply-To: <20120509175758.6a613706@sacrilege> X-Google-Sender-Auth: VlUlH_LDXRQ7qAkYGEHKbt-SGdM X-Gm-Message-State: ALoCoQmbMxX3SdpSlrRpLes/Cba44mSirP5Q/UFB+jh+IWtsT41Ln2oYR2IfChlljXhNXFTJZ9Rk X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 209.85.214.41 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Original-Sender: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.devel:150393 Archived-At: --0015175cf764a3399504bf9c2c49 Content-Type: text/plain; charset=UTF-8 Explanation and patch make sense, +1 from me. On Wed, May 9, 2012 at 4:57 AM, Mike Kazantsev wrote: > Good day, > > Patch is a fix for a regression, which was introduced by the patch I > previously posted on this list (Date: Mon, 31 Oct 2011 09:56:58 +0600, > Subject: [PATCH] erc-dcc: allow SEND commands containing quoted > filenames with spaces in them). > > As I see, it was picked up by Julien on Mon Nov 28 10:24:08 2011 +0100 > in bb90806bc156833c (git://git.savannah.gnu.org/emacs.git). > > Unfortunately, during one of the iterations of regexp-mangling, I > forgot to update match group number for *unquoted* filename matches, > thus introducing the regression. > > Regexp in question is erc-dcc-ctcp-query-send-regexp and currently it > looks like this: > > (concat "^DCC SEND \\(" > ;; Following part matches either filename without spaces > ;; or filename enclosed in double quotes with any number > ;; of escaped double quotes inside. > "\"\\(\\(.*?\\(\\\\\"\\)?\\)+?\\)\"\\|\\([^ ]+\\)" > "\\) \\([0-9]+\\) \\([0-9]+\\) *\\([0-9]*\\)") > > And the match is dissected like this: > > (let > ((filename > (or (match-string 3 query) > (erc-dcc-unquote-filename (match-string 2 query)))) > (ip (erc-decimal-to-ip (match-string 6 query))) > ... > > As can be seen, though, unquoted filename is only matched by the fifth > regexp group ("|\\([^ ]+\\)" part), not third, thus matching of > unquoted filenames fails with non-descriptive nil-instead-of-string > error. > > Issue was masked until now for me by custom (simplier hack) > erc-dcc-ctcp-query-send-regexp definition in my config, older emacs > version and relatively infrequent use of dcc with clients that don't > use filename-quoting. > > Apologies to Julien and Michael for unasked-for CC, but I thought you > might be interested and should be notified, as people who reviewed the > initial patch. > > Sorry for a lousy job of testing previous patch. > Thanks. > > > From e5fe5e71b0c7a898132dfce2a7ec72b8703079bd Mon Sep 17 00:00:00 2001 > From: Mike Kazantsev > Date: Wed, 9 May 2012 17:27:27 +0600 > Subject: [PATCH] etc-dcc: Fix handling SEND commands with unquoted > filename. > > * erc-dcc.el (erc-dcc-handle-ctcp-send): Fix regexp match group numbers. > > --- > lisp/erc/erc-dcc.el | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/lisp/erc/erc-dcc.el b/lisp/erc/erc-dcc.el > index ba87cf6..d1ef1a9 100644 > --- a/lisp/erc/erc-dcc.el > +++ b/lisp/erc/erc-dcc.el > @@ -674,7 +674,7 @@ It extracts the information about the dcc request and > adds it to > ?r "SEND" ?n nick ?u login ?h host)) > ((string-match erc-dcc-ctcp-query-send-regexp query) > (let ((filename > - (or (match-string 3 query) > + (or (match-string 5 query) > (erc-dcc-unquote-filename (match-string 2 query)))) > (ip (erc-decimal-to-ip (match-string 6 query))) > (port (match-string 7 query)) > -- > 1.7.10 > > > -- > Mike Kazantsev // fraggod.net > -- Michael Olson | http://mwolson.org/ --0015175cf764a3399504bf9c2c49 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Explanation and patch make sense, +1 from me.

On Wed, May 9, 2012 at 4:57 AM, Mike Kazantsev <<= a href=3D"mailto:mk.fraggod@gmail.com" target=3D"_blank">mk.fraggod@gmail.c= om> wrote:
Good day,

Patch is a fix for a regression, which was introduced by the patch I
previously posted on this list (Date: Mon, 31 Oct 2011 09:56:58 +0600,
Subject: [PATCH] erc-dcc: allow SEND commands containing quoted
filenames with spaces in them).

As I see, it was picked up by Julien on Mon Nov 28 10:24:08 2011 +0100
in bb90806bc156833c (git://git.savannah.gnu.org/emacs.git).

Unfortunately, during one of the iterations of regexp-mangling, I
forgot to update match group number for *unquoted* filename matches,
thus introducing the regression.

Regexp in question is erc-dcc-ctcp-query-send-regexp and currently it
looks like this:

=C2=A0(concat "^DCC SEND \\("
=C2=A0 ;; Following part matches either filename without spaces
=C2=A0 ;; or filename enclosed in double quotes with any number
=C2=A0 ;; of escaped double quotes inside.
=C2=A0 "\"\\(\\(.*?\\(\\\\\"\\)?\\)+?\\)\"\\|\\([^ ]+\= \)"
=C2=A0 "\\) \\([0-9]+\\) \\([0-9]+\\) *\\([0-9]*\\)")

And the match is dissected like this:

=C2=A0(let
=C2=A0 =C2=A0((filename
=C2=A0 =C2=A0 =C2=A0(or (match-string 3 query)
=C2=A0 =C2=A0 =C2=A0 =C2=A0(erc-dcc-unquote-filename (match-string 2 query= ))))
=C2=A0 =C2=A0(ip (erc-decimal-to-ip (match-string 6 query)))
=C2=A0 =C2=A0...

As can be seen, though, unquoted filename is only matched by the fifth
regexp group ("|\\([^ ]+\\)" part), not third, thus matching of unquoted filenames fails with non-descriptive nil-instead-of-string
error.

Issue was masked until now for me by custom (simplier hack)
erc-dcc-ctcp-query-send-regexp definition in my config, older emacs
version and relatively infrequent use of dcc with clients that don't use filename-quoting.

Apologies to Julien and Michael for unasked-for CC, but I thought you
might be interested and should be notified, as people who reviewed the
initial patch.

Sorry for a lousy job of testing previous patch.
Thanks.


>From e5fe5e71b0c7a898132dfce2a7ec72b8703079bd Mon Sep 17 00:00:00 2001
From: Mike Kazantsev <mk.fraggod= @gmail.com>
Date: Wed, 9 May 2012 17:27:27 +0600
Subject: [PATCH] etc-dcc: Fix handling SEND commands with unquoted filename= .

* erc-dcc.el (erc-dcc-handle-ctcp-send): Fix regexp match group numbers.
---
=C2=A0lisp/erc/erc-dcc.el | =C2=A0 =C2=A02 +-
=C2=A01 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lisp/erc/erc-dcc.el b/lisp/erc/erc-dcc.el
index ba87cf6..d1ef1a9 100644
--- a/lisp/erc/erc-dcc.el
+++ b/lisp/erc/erc-dcc.el
@@ -674,7 +674,7 @@ It extracts the information about the dcc request and a= dds it to
=C2=A0 =C2=A0 =C2=A0 =C2=A0?r "SEND" ?n nick ?u login ?h host))<= br> =C2=A0 =C2=A0 =C2=A0((string-match erc-dcc-ctcp-query-send-regexp query) =C2=A0 =C2=A0 =C2=A0 (let ((filename
- =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (or (match-string 3 query)
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (or (match-string 5 query)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(erc-dcc-unq= uote-filename (match-string 2 query))))
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (ip =C2=A0 =C2=A0 =C2=A0 (erc-de= cimal-to-ip (match-string 6 query)))
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (port =C2=A0 =C2=A0 (match-strin= g 7 query))
--
1.7.10


--
Mike Kazantsev // fraggod.= net



-- Michael Olson=C2=A0 |=C2=A0 http://mwolson.org/
--0015175cf764a3399504bf9c2c49--