unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
From: Fabio Natali via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
To: Eli Zaretskii <eliz@gnu.org>
Cc: 74218@debbugs.gnu.org
Subject: bug#74218: [PATCH] Ask confirmation before sending region to search engine.
Date: Wed, 06 Nov 2024 15:27:04 +0000	[thread overview]
Message-ID: <87ikt0gz7b.fsf@fabionatali.com> (raw)
In-Reply-To: <86pln8sfqe.fsf@gnu.org>

[-- Attachment #1: Type: text/plain, Size: 2794 bytes --]

Hi Eli,

Please find attached a v2 that - hopefully - addresses the points
mentioned in your email. Please see my further comments inline below.

Thanks for all the help, cheers, Fabio.

On 2024-11-06, 14:34 +0200, Eli Zaretskii <eliz@gnu.org> wrote:
>> * lisp/net/eww.el (eww-search-confirm-send-region,
>> eww-search-words): With 'eww-search-words' (by default bound to 'M-s
>> M-w') a user can type in some search terms and get back the results
>> of a web search from a predefined search engine. If a region is
>> selected, 'eww-search-words' will use that for the web search
>> instead of prompting the user.
>
> This should be reformatted according to our conventions, see
> CONTRIBUTE.

Ok, here's what I've changed:

- Set max line length to 63 chars.
- Slightly reordered the text so that some broader explanation comes
  first and the ChangeLog entries later.
- Micro-improvements to the ChangeLog entries.

I hope it looks better now - but I'm still a little unsure. If there's
anything else that's left to fix, please let me know.

> The first line of a doc string should be a single complete sentence,
> and should attempt to summarize what the function/variable does,
> because some "apropos" commands show only the first line of each doc
> string.

Ha! True, sorry, that's also fixed now.

>> +  :version "30.0"
>
> This should be "31.1".

Fixed.

>> +                 (format-message
>> +                  "Send region to the configured search engine? ")))
>
> IMO, this should somehow try to indicate the problematic aspect of
> doing this.  For example, maybe it should say
>
>        Really send the entire region to the search engine?

Good one, fixed.

> It is also possible that short regions should be sent without any need
> for confirmation.  In which case perhaps the variable should allow
> integer values, not just nil and t.

I think I disagree on this one.

The functionality you suggest is a superset of what I implemented and it
goes in the direction of giving more freedom to the user. On the other
hand, however, I don't see a strong correlation between the sensitivity
of a piece of information and its length.

For the sake of simplicity, I'd have a preference to maintain the
boolean logic as per my original patch.

> In addition, I don't see any need to ask for confirmation when we are
> not going to send anything to the search engine, so I think the test
> for white-space region should be before the confirmation prompt, and
> only if the region is going to be sent.

Ha, another good one! Thanks, fixed.

> Would you like to start at this time your legal paperwork of assigning
> the copyright to the FSF, so that we could accept your future
> contributions without limitations?

Sent separately, thanks.


-- 
Fabio Natali
https://fabionatali.com



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: v2-0001-Ask-confirmation-before-sending-region-to-search-.patch --]
[-- Type: text/x-patch, Size: 2654 bytes --]

From cdd17053befac8298a04d0cdfc4cafe5a410166b Mon Sep 17 00:00:00 2001
From: Fabio Natali <me@fabionatali.com>
Date: Tue, 5 Nov 2024 23:52:30 +0000
Subject: [PATCH v2] Ask confirmation before sending region to search engine

With 'eww-search-words' (by default bound to 'M-s M-w') a user
can type in some search terms and get back the results of a web
search from a predefined search engine. If a region is selected,
'eww-search-words' will use that for the web search instead of
prompting the user.

In its current form, 'eww-search-words' presents a security and
usability problem. It is relatively too easy to mistakenly
launch the function and, if a region of text is selected, have
potentially sensitive data sent out to a third-party service.

This commit changes the search function's default behaviour so
that explicit confirmation is required before a region is sent
to a search engine. The behaviour can be adjusted via the
newly-introduced 'eww-search-confirm-send-region' variable,
which is set to true by default.

* lisp/net/eww.el (eww-search-confirm-send-region): Add.
(eww-search-words): Update default 'eww-search-words' behaviour
so as to ask confirmation before sending a region to a search
engine.
---
 lisp/net/eww.el | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/lisp/net/eww.el b/lisp/net/eww.el
index 2d351dff88f..cbf989f4a6a 100644
--- a/lisp/net/eww.el
+++ b/lisp/net/eww.el
@@ -52,6 +52,17 @@
   :group 'eww
   :type 'string)
 
+(defcustom eww-search-confirm-send-region t
+  "Whether to confirm before sending a region to a search engine.
+Non-nil if EWW should ask confirmation before sending the
+selected region to the configured search engine.  This is the
+default to mitigate the risk of accidental data leak.  Set this
+variable to nil to send the region to the search engine
+straightaway."
+  :version "31.1"
+  :group 'eww
+  :type 'boolean)
+
 (defcustom eww-search-prefix "https://duckduckgo.com/html/?q="
   "Prefix URL to search engine."
   :version "24.4"
@@ -605,7 +616,12 @@ for the search engine used."
   (if (use-region-p)
       (let ((region-string (buffer-substring (region-beginning) (region-end))))
         (if (not (string-match-p "\\`[ \n\t\r\v\f]*\\'" region-string))
-            (eww region-string)
+            (when
+                (or (not eww-search-confirm-send-region)
+                    (yes-or-no-p
+                     (format-message
+                      "Really send the entire region to the search engine? ")))
+              (eww region-string))
           (call-interactively #'eww)))
     (call-interactively #'eww)))
 
-- 
2.46.0


  parent reply	other threads:[~2024-11-06 15:27 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-11-06  0:46 bug#74218: [PATCH] Ask confirmation before sending region to search engine Fabio Natali via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-11-06 12:34 ` Eli Zaretskii
2024-11-06 13:18   ` Fabio Natali via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-11-06 13:38     ` Eli Zaretskii
2024-11-06 15:27   ` Fabio Natali via Bug reports for GNU Emacs, the Swiss army knife of text editors [this message]
2024-11-07  1:51     ` Stefan Kangas
2024-11-07  8:42       ` Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-11-07  8:53         ` Eli Zaretskii
2024-11-07  9:02           ` Robert Pluim
2024-11-07 10:49             ` Eli Zaretskii
2024-11-07 11:03               ` Robert Pluim
2024-11-07 11:05                 ` Eli Zaretskii
2024-11-07 11:19                   ` Robert Pluim
2024-11-07 11:29                   ` Fabio Natali via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-11-07 11:56                     ` Eli Zaretskii
2024-11-07 14:04                       ` Fabio Natali via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-11-07  9:12           ` Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-11-07 10:52             ` Eli Zaretskii

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87ikt0gz7b.fsf@fabionatali.com \
    --to=bug-gnu-emacs@gnu.org \
    --cc=74218@debbugs.gnu.org \
    --cc=eliz@gnu.org \
    --cc=me@fabionatali.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).