[16.0] [MIG] partner_identification

[marielejeune]

No description provided.

[rousseldenis]

/ocabot migration partner_identification

[OCA-git-bot]

The migration issue (#1353) has been updated to reference the current pull request.
however, a previous pull request was referenced : #1374.
Perhaps you should check that there is no duplicate work.
CC : @deu-dev

[bealdav]

I have no strong opinion on this matter but OCA/server-tools#1941 (comment) suggest format is dangerous.

https://github.com/OCA/odoo-community.org/blob/master/website/Contribution/CONTRIBUTING.rst
in Idioms § says "Prefer % over .format(), prefer %(varname) instead of positional"

Should we improve guidelines ? or use %(varname) with dict ?
cc @rousseldenis

[rousseldenis]

We can open an issue to face pros and cons of each solution (format or %).

That said, it would be very uncommon to have a translator with that kind of will. But maybe, letting an open door...

@sbidoul @yajo

[sbidoul]

I've looked at that article about the safety of format again, and in this case, there is no risk since all variables passed to format are of type str. So to me this code is fine.

[rousseldenis]

So, do we agree, as I thought also about str type, that format() can be used for translations ?

[sbidoul]

My understanding is that it is at risk (against hostile translators) if python objects are passed to format(). So this requires careful review that only simple types are passed to it.

[yajo]

Yes, this is a security risk and must be reverted.

[sbidoul]

Please elaborate, as I don't see the risk when strings are passed as format variables.

[yajo]

The decision was taken here: OCA/pylint-odoo#302. It contains some links to in-depth explanations.

In this particular case, even if it could be demonstrated that it's not a security problem, it definitely opens the door for more foot-shooting than the other format. And I guess it will confuse translators unnecessarily (all of them are used to %s-style placeholders instead).

Some abuse examples:

>>> "{innocent_string}".format(innocent_string="hellö") # valid
'hellö'
>>> "{not_so_innocent_string!a}".format(not_so_innocent_string="hellö")
"'hell\\xf6'"
>>> "{not_so_innocent_string!i}".format(not_so_innocent_string="hellö")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ValueError: Unknown conversion specifier i
>>> "{not_so_innocent_string!d}".format(not_so_innocent_string="hellö")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ValueError: Unknown conversion specifier d
>>> "{not_so_innocent_string.__class__}".format(not_so_innocent_string="hellö")
"<class 'str'>"
>>> "{not_so_innocent_string.__class__.__dict__}".format(not_so_innocent_string="hellö")
'{\'__new__\': <built-in method __new__ of type object at 0x7fab9d0ed760>, \'__repr__\': <slot wrapper \'__repr__\' of \'str\' objects>, \'__hash__\': <slot wrapper \'__hash__\' of \'str\' objects>, \'__str__\': <slot wrapper \'__str__\' of \'str\' objects>, \'__getattribute__\': <slot wrapper \'__getattribute__\' of \'str\' objects>, \'__lt__\': <slot wrapper \'__lt__\' of \'str\' objects>, \'__le__\': <slot wrapper \'__le__\' of \'str\' objects>, \'__eq__\': <slot wrapper \'__eq__\' of \'str\' objects>, \'__ne__\': <slot wrapper \'__ne__\' of \'str\' objects>, \'__gt__\': <slot wrapper \'__gt__\' of \'str\' objects>, \'__ge__\': <slot wrapper \'__ge__\' of \'str\' objects>, \'__iter__\': <slot wrapper \'__iter__\' of \'str\' objects>, \'__mod__\': <slot wrapper \'__mod__\' of \'str\' objects>, \'__rmod__\': <slot wrapper \'__rmod__\' of \'str\' objects>, \'__len__\': <slot wrapper \'__len__\' of \'str\' objects>, \'__getitem__\': <slot wrapper \'__getitem__\' of \'str\' objects>, \'__add__\': <slot wrapper \'__add__\' of \'str\' objects>, \'__mul__\': <slot wrapper \'__mul__\' of \'str\' objects>, \'__rmul__\': <slot wrapper \'__rmul__\' of \'str\' objects>, \'__contains__\': <slot wrapper \'__contains__\' of \'str\' objects>, \'encode\': <method \'encode\' of \'str\' objects>, \'replace\': <method \'replace\' of \'str\' objects>, \'split\': <method \'split\' of \'str\' objects>, \'rsplit\': <method \'rsplit\' of \'str\' objects>, \'join\': <method \'join\' of \'str\' objects>, \'capitalize\': <method \'capitalize\' of \'str\' objects>, \'casefold\': <method \'casefold\' of \'str\' objects>, \'title\': <method \'title\' of \'str\' objects>, \'center\': <method \'center\' of \'str\' objects>, \'count\': <method \'count\' of \'str\' objects>, \'expandtabs\': <method \'expandtabs\' of \'str\' objects>, \'find\': <method \'find\' of \'str\' objects>, \'partition\': <method \'partition\' of \'str\' objects>, \'index\': <method \'index\' of \'str\' objects>, \'ljust\': <method \'ljust\' of \'str\' objects>, \'lower\': <method \'lower\' of \'str\' objects>, \'lstrip\': <method \'lstrip\' of \'str\' objects>, \'rfind\': <method \'rfind\' of \'str\' objects>, \'rindex\': <method \'rindex\' of \'str\' objects>, \'rjust\': <method \'rjust\' of \'str\' objects>, \'rstrip\': <method \'rstrip\' of \'str\' objects>, \'rpartition\': <method \'rpartition\' of \'str\' objects>, \'splitlines\': <method \'splitlines\' of \'str\' objects>, \'strip\': <method \'strip\' of \'str\' objects>, \'swapcase\': <method \'swapcase\' of \'str\' objects>, \'translate\': <method \'translate\' of \'str\' objects>, \'upper\': <method \'upper\' of \'str\' objects>, \'startswith\': <method \'startswith\' of \'str\' objects>, \'endswith\': <method \'endswith\' of \'str\' objects>, \'removeprefix\': <method \'removeprefix\' of \'str\' objects>, \'removesuffix\': <method \'removesuffix\' of \'str\' objects>, \'isascii\': <method \'isascii\' of \'str\' objects>, \'islower\': <method \'islower\' of \'str\' objects>, \'isupper\': <method \'isupper\' of \'str\' objects>, \'istitle\': <method \'istitle\' of \'str\' objects>, \'isspace\': <method \'isspace\' of \'str\' objects>, \'isdecimal\': <method \'isdecimal\' of \'str\' objects>, \'isdigit\': <method \'isdigit\' of \'str\' objects>, \'isnumeric\': <method \'isnumeric\' of \'str\' objects>, \'isalpha\': <method \'isalpha\' of \'str\' objects>, \'isalnum\': <method \'isalnum\' of \'str\' objects>, \'isidentifier\': <method \'isidentifier\' of \'str\' objects>, \'isprintable\': <method \'isprintable\' of \'str\' objects>, \'zfill\': <method \'zfill\' of \'str\' objects>, \'format\': <method \'format\' of \'str\' objects>, \'format_map\': <method \'format_map\' of \'str\' objects>, \'__format__\': <method \'__format__\' of \'str\' objects>, \'maketrans\': <staticmethod(<built-in method maketrans of type object at 0x7fab9d0ed760>)>, \'__sizeof__\': <method \'__sizeof__\' of \'str\' objects>, \'__getnewargs__\': <method \'__getnewargs__\' of \'str\' objects>, \'__doc__\': "str(object=\'\') -> str\\nstr(bytes_or_buffer[, encoding[, errors]]) -> str\\n\\nCreate a new string object from the given object. If encoding or\\nerrors is specified, then the object must expose a data buffer\\nthat will be decoded using the given encoding and error handler.\\nOtherwise, returns the result of object.__str__() (if defined)\\nor repr(object).\\nencoding defaults to sys.getdefaultencoding().\\nerrors defaults to \'strict\'."}'

OTOH, since the current standard is the contrary, it should be you who elaborates why we need this exception, don't you think? 😅

[sbidoul]

So merging.

If a practical security issue is raised later, we can still update.

/ocabot merge patch

[OCA-git-bot]

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 16.0-ocabot-merge-pr-1397-by-sbidoul-bump-patch, awaiting test results.

[OCA-git-bot]

Congratulations, your PR was merged at ccddeb0. Thanks a lot for contributing to OCA. ❤️