You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Yasuhito FUTATSUKI <fu...@yf.bsdclub.org> on 2021/05/08 18:17:44 UTC

[PATCH] Fix file path transcoding around editor invocation.

Hi,

Jun found that an error message on failing editor invocation on
commit/propchange or edit to resolve confilict is still corrupted if
editor or file name contains non-ascii character and not in UTF-8 
locale even after r1887641 on Windows (and perhaps on macOS).

To reproduce it, in non UTF-8 CodePage/locale, set SVN_EDITOR to
non existent path which contains non-ascii character and

* Try to "svn propedit" or "svn commit"
   (svn_cmdline__edit_string_externally() code path)
* Make conflict on file which name is contain non-ascii character
  and then try to resolve by editing file from svn command
   (svn_cmdine__edit_file_externally() code path)

It was caused by wrong usage of the transcode functions: I used the
transcode functions for console I/O and messages to file paths.

Here is a patch address it. Could anyone please review it?

[[[
Follow-up to r1887641: Fix file path transcoding around editor invocation.

On r1887641, we use svn_utf_cstring_to_utf8() to transcode the command
line string from native file path on OS into UTF-8. However as svn_utf_
functions are for console/file I/O, it was incorrect and actually it
caused string corruption if editor invocation had failed on Windows
(and perhaps macOS) platform.

With this commit we use svn_path_cstring_to_utf8() and 
svn_path_cstring_from_utf8() for transcoding file paths instead. This
fixes the string corruption above, and also fix potential string
corruption on transcoding temporary filename, which is always ascii
string and safe on encodings we handle in current implementation.

* subversion/libsvn_subr/cmdline.c
  (svn_cmdline__edit_file_externally):
    Use svn_path_cstring_to_utf8 to transcode file paths.
  (svn_cmdline__edit_string_externally):
    Use svn_path_cstring_from_utf8 and svn_path_cstring_to_utf8 to
    transcode file paths.

Found by: jun66j5
]]]


Thanks,
-- 
Yasuhito FUTATSUKI <fu...@yf.bsclub.org>

Re: [PATCH] Fix file path transcoding around editor invocation.

Posted by Yasuhito FUTATSUKI <fu...@yf.bsdclub.org>.
On 2021/05/18 12:46, Jun Omae wrote:
> Hi,
> 
> On Sun, May 9, 2021 at 3:18 AM Yasuhito FUTATSUKI
> <fu...@yf.bsdclub.org> wrote:
>> Jun found that an error message on failing editor invocation on
>> commit/propchange or edit to resolve confilict is still corrupted if
>> editor or file name contains non-ascii character and not in UTF-8
>> locale even after r1887641 on Windows (and perhaps on macOS).
>>
>> To reproduce it, in non UTF-8 CodePage/locale, set SVN_EDITOR to
>> non existent path which contains non-ascii character and
> 
> The patch looks good to me and verified with the following environments:
> 
>  * Windows 10 (cp932)
>  * Linux (ja_JP.eucjp)
 
Thank you very much for the review and verification.

Commited in r1890013 and added to r1887641 group in 1.14.x back port
nomination.

Cheers,
-- 
Yasuhito FUTATSUKI <fu...@yf.bsclub.org>

Re: [PATCH] Fix file path transcoding around editor invocation.

Posted by Jun Omae <ju...@gmail.com>.
Hi,

On Sun, May 9, 2021 at 3:18 AM Yasuhito FUTATSUKI
<fu...@yf.bsdclub.org> wrote:
> Jun found that an error message on failing editor invocation on
> commit/propchange or edit to resolve confilict is still corrupted if
> editor or file name contains non-ascii character and not in UTF-8
> locale even after r1887641 on Windows (and perhaps on macOS).
>
> To reproduce it, in non UTF-8 CodePage/locale, set SVN_EDITOR to
> non existent path which contains non-ascii character and

The patch looks good to me and verified with the following environments:

 * Windows 10 (cp932)
 * Linux (ja_JP.eucjp)

-- 
Jun Omae <ju...@gmail.com> (大前 潤)