Re: svn commit: r1877712 - in /subversion/trunk/subversion/tests/cmdline: entries-dump.c svntest/main.py

On 2020/05/14 22:55, Daniel Shahaf wrote:
> futatuki_at_apache.org wrote on Thu, 14 May 2020 01:46 -0000:
>> Log:
>> Use safe bytes literals when set string values in working copy entries.
>>
>
> If someone reads this log message in a year, I don't think it'd be clear
> to them what the problem being fixed was. I think it would be useful to
> be more specific about the circumstances; for example:>
> entries-dump: Escape string-typed attribute values when serializing> them as Python string literals.
>
> Before this commit, a filesystem node named «foo\bar» (a single,
> 7-character path component) would cause «e.name = 'foo\bar'» to be
> emitted. The unescaped backslash would manifest as a test failure or
> a SyntaxError, depending on the following characters.
>
> This was triggered by svnadmin_tests 42 verify_metadata_only under
> Python 3 on Windows.
>
> The reference to "svnadmin_tests 42 verify_metadata_only" is only
> a placeholder, to be replaced by the correct reference.
>
> I specifically used the term "filesystem node" to clarify that «foo\bar»
> was to be taken as an svn_fspath__* path (where backslash has no
> special meaning), as opposed to, say, an svn_dirent_* path (where
> backslash is a delimiter on Windows).

Thank you for the review and lesson on my commit message. I intended
to send this commit message with the patch before commit, but missed
by mistakes.

I added note about quotation characters which could also cause SyntaxError.

Before replace this log message, I'd like to get a review again.

[[[
entries-dump: Escape string-typed attribute values when serializing
them as Python string literals.

Before this commit, a filesystem node named "foo\bar" (a single,
7-character path component) would cause "e.name = 'foo\bar'" to be
emitted. The unescaped backslash would manifest as a test failure or
a SyntaxError, depending on the following characters.

This was triggered by update_tests.py 76 windows_update_backslash under
Python 3 on Windows.

Also, user names can contain "'" (a single quote character) and/or """
(a double quote character), which would potentially cause a SyntaxError
even if we choose ether of them to quote string literals.

* subversion/tests/cmdline/entries-dump.c
  (print_prefix): New function.
  (str_value):
  - Add argument to specify pool.
  - Print human readable value of "value" as is in comment, then set it
    as str value by using hex escaped bytes literal.
  (entries_dump): Add pool argument to str_value() calls.
  (main):
  - Print "Entry" class definition as prefix before entry_dump() or tree_dump()
  - Style fix on if statement (using blocks).
  (): Add include files for assert() and svn_xml_escape_attr_cstring()
* subversion/tests/cmdline/svntest/main.py
  (run_entiresdump, run_entriesdump_tree): Move definition of "Entry" class
  into generated code by entries-dump execution.

Found by: svn-windows-ra buildbot
Review by: danielsh
]]]
 
Thanks,

-- 
Yasuhito FUTATSUKI <futatuki_at_poem.co.jp>/<futatuki_at_yf.bsdclub.org>