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

Temp file encoding of curl payloads #77

Closed
gariepyalex opened this issue May 22, 2017 · 9 comments · Fixed by #128
Closed

Temp file encoding of curl payloads #77

gariepyalex opened this issue May 22, 2017 · 9 comments · Fixed by #128
Milestone

Comments

@gariepyalex
Copy link

When send an HTTP payload using curl, emacs-request make use of a temp file for the content of the payload. However, when the payloads contains non-ascii characters, the temporary file is encoded in latin1 instead of in utf-8.

The problem seems to come from there:
fb93d11

Changing the code to:

(let ((file-coding-system-alist nil)
             (coding-system-for-write 'utf-8))
         (with-temp-file tempfile
           (setq buffer-file-coding-system 'utf-8)
           (insert data)))

seems to fix the problem. I can open a pull request if you want.

@HIRANO-Satoshi
Copy link

Hi,

We have this issue with org-trello and all of non-English users could not use it.

@gariepyalex Could you kindly send a PR?

gariepyalex added a commit to gariepyalex/emacs-request that referenced this issue Jun 28, 2017
@MaksVal
Copy link

MaksVal commented Oct 19, 2017

Hi!

Sorry, but your workaround isn't working for me... I changed your code a litle:

(setq buffer-file-coding-system 'utf-8)
       (insert data)
       (decode-coding-region (point-min) (point-max) 'utf-8)))

@titaniumbones
Copy link
Collaborator

Is there an alternative version of the request package that can be installed from melpa or somewhere else? This fix seems essential; I have stored the defun of request--curl-command in my init file (as part of the :config directive in a use-package invocation), but it seems like the better option would be for someone else to take over maintenance of the project. @DamienCassou have you thought about sharing your fork?

@DamienCassou
Copy link
Contributor

DamienCassou commented Nov 28, 2018 via email

@titaniumbones
Copy link
Collaborator

Does #85 fix this issue for you @gariepyalex and @MaksVal ?

@titaniumbones titaniumbones added the need info Can't address the problem wihtout more information label Nov 30, 2018
@titaniumbones titaniumbones added this to the 0.40 milestone Nov 30, 2018
@MaksVal
Copy link

MaksVal commented Apr 16, 2019

@titaniumbones this is a fine solution! Thanks!

@rogorido
Copy link

rogorido commented Jul 26, 2019

What is exactly the status of this issue? I have an issue with anki-editor (louietan/anki-editor#51) which seems related to this... (I'm using request 0.3.1, but with master I had the same problem)

dickmao pushed a commit to dickmao/emacs-request that referenced this issue Jul 26, 2019
While we still heavy-handedly default the encoding to utf-8, we fall
back to no-conversion if that assumption fails (rather than allowing
`select-safe-coding-system` to bring up a confusing dialogue).

Closes tkf#77
@dickmao
Copy link
Collaborator

dickmao commented Jul 26, 2019

@rogorido We made a backwards incompatible change in release 0.3.1. louietan/anki-editor#53 updates your package to be compatible with 0.3.1. #128 attempts to mitigate other fallouts of this kind.

@dickmao dickmao removed the need info Can't address the problem wihtout more information label Jul 26, 2019
@MaksVal
Copy link

MaksVal commented Jul 27, 2019 via email

dickmao pushed a commit to dickmao/emacs-request that referenced this issue Nov 14, 2019
Commit 1562f9c seems to conflate the desire to encode request data in
utf8 with also decoding server output.

In light of tkf#157 and tkf#158, I am removing utf-8 decoding altogether.

The impetus to encode to utf-8 in tkf#77, tkf#85, and
github/org-trello/#340 suggest nothing about also decoding in utf-8,
so I am crossing my fingers I won't rebreak for them.

Also, clean up logging.  It was impossible to follow with all the
capital letters.
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

Successfully merging a pull request may close this issue.

7 participants