parkouss / webmacs

webmacs - keyboard driven (emacs key bindings) browser, https://webmacs.readthedocs.io/en/latest/
GNU General Public License v3.0
156 stars 21 forks source link

Crashes when using webjump with URL that contains (unescaped) percent signs #67

Closed xantoz closed 6 years ago

xantoz commented 6 years ago

e.g: define_webjump("emacswiki", "https://startpage.com/do/search?cat=web&cmd=process_search&language=english&engine0=v1all&query=%s%20site%3Aemacswiki.org&abp=-1")

If you try to use this webjump webmacs crashes with the following traceback:

Traceback (most recent call last):
  File "/path/to/webmacs/webmacs/commands/__init__.py", line 47, in call
    self.binding(ctx)
  File "/path/to/webmacs/webmacs/commands/webjump.py", line 429, in go_to
    url = get_url(ctx.prompt)
  File "/path/to/webmacs/webmacs/commands/webjump.py", line 399, in get_url
    "utf-8")
TypeError: not enough arguments for format string
Aborted

This used to work at least as recently as commit ae30c34ae888a047047101836443bb2f6783e502. It also used to work the same way in conkeror, that it only treats the sequence "%s" as special and another other sequence with "%" is included verbatim.

I understand that it might be preferrable to require escaping of '%' using "%%", but I think that it should probably be an error message when trying to use it instead of crashing. Alternatively it could also error out early when the webjumps are being defined to help catch badly formatted webjump definitions early.

parkouss commented 6 years ago

Maybe just replacing %s - without modifying any other % in url - would be enough then?

Since conkeror did that apparently, and according to https://en.wikipedia.org/wiki/Percent-encoding I think this should be sufficient.

Do you agree?

parkouss commented 6 years ago

Fix according to my previous comment: PR #76.