Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ampersands and equal signs break parsing #8

Open
sllvn opened this issue Jul 22, 2019 · 3 comments
Open

Ampersands and equal signs break parsing #8

sllvn opened this issue Jul 22, 2019 · 3 comments

Comments

@sllvn
Copy link

sllvn commented Jul 22, 2019

First off, thanks for this util! ❤️

I've noticed that titles with ampersands are breaking the parser, e.g.

python parse.py 'org-protocol://capture?template=w&url=https%3A%2F%2Fexample.com&title=foo%20%26%20bar'

should send the title "foo & bar" through, but only characters up to the ampersand are sent ("foo " with a trailing space). This is also the case with equal signs (=).

I've worked around this by modifying parse.py but I'm not sure this is correct. Also not sure if there are other characters that cause parsing to break.

 raw_url = six.moves.urllib.parse.unquote(url.replace('%26', '%2526').replace('%3D', '%253D'))

Thanks!

@aaronbieber
Copy link
Owner

I am not set up to be able to easily test this at the moment. I suspect that your solution of double-encoding the characters is not correct, however. I'm actually uncertain if unquote-ing the URL passed to emacsclient is the right thing to do here based purely on the docs. It may be that the full URL needs to be shell escaped instead.

If you feel up to giving that a try, I'd be very grateful.

@sllvn
Copy link
Author

sllvn commented Jul 29, 2019

Took a second look and things work with no unquoteing of the url, i.e.

url = sys.argv[1]
config = read_config()
cmd = emacs_client_command(config)
cmd.append(url)
subprocess.check_output(cmd)

appears to work.

You don't happen to remember why unquote was originally included, do you? If we need to shell-escape, we can pass shell=True to subprocess.check_output, but that comes with a security caveat: https://docs.python.org/2/library/subprocess.html#frequently-used-arguments

If we don't think there's a reason unquote needs to remain, I'm happy to submit a PR to remove it.

@malcolmpurvis
Copy link

I can confirm that the unquote in main isn't required and the original URL needs to be passed on to Emacs.

With the unquote present YouTube URLs are truncated, amongst other problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants