[issue29190] Avoid possible errors in comparing strings

2017-01-09 Thread Stefan Krah

Stefan Krah added the comment:

On Mon, Jan 09, 2017 at 08:21:17AM +, Serhiy Storchaka wrote:
> In the particular case of getround() in _decimal.c, seems the worst case is 
> raising TypeError instead of MemoryError in pretty rare circumstances. This 
> is not critically bad, there are a lot of other places where the initial 
> exception is silently replaced by less specific exception. But the code 
> *looks* fragile.

No, it does not.  It is obvious to a human that -1 <==> "not equal".

If Coccinelle does not understand that, well ...

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue29190] Avoid possible errors in comparing strings

2017-01-09 Thread Serhiy Storchaka

Serhiy Storchaka added the comment:

In the particular case of getround() in _decimal.c, seems the worst case is 
raising TypeError instead of MemoryError in pretty rare circumstances. This is 
not critically bad, there are a lot of other places where the initial exception 
is silently replaced by less specific exception. But the code *looks* fragile.

--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue29190] Avoid possible errors in comparing strings

2017-01-09 Thread Roundup Robot

Roundup Robot added the comment:

New changeset 337461574c90 by Serhiy Storchaka in branch '3.5':
Issue #29190: Fixed possible errors in comparing strings in the pickle module.
https://hg.python.org/cpython/rev/337461574c90

New changeset 9fcff936f61f by Serhiy Storchaka in branch '3.6':
Issue #29190: Fixed possible errors in comparing strings in the pickle module.
https://hg.python.org/cpython/rev/9fcff936f61f

New changeset f477c715076c by Serhiy Storchaka in branch 'default':
Issue #29190: Fixed possible errors in comparing strings in the pickle module.
https://hg.python.org/cpython/rev/f477c715076c

--
nosy: +python-dev

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue29190] Avoid possible errors in comparing strings

2017-01-07 Thread Stefan Krah

Stefan Krah added the comment:

I'm generally a little concerned about the way "bugs" are presented here 
recently:

In #28701 you write:

'Correctness. Since no caller checks the error of 
PyUnicode_CompareWithASCIIString(), it is incorrectly interpreted as "less 
then".'

This is just not true. When testing for equality "-1" is "not equal"
and an exception would follow anyway.

I'm also not happy with broad coccinelle patches that I see months later.

I'm not sure if you realize that other people's reputation is at
stake here. You label something as a bug (good for you), everyone
who reads your reports and commits think that a bug has been found,
which is not the case.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue29190] Avoid possible errors in comparing strings

2017-01-07 Thread Stefan Krah

Stefan Krah added the comment:

To expand a little, you use the terminology "possible bugs". All I can
see is a double exception if PyUnicode_Compare() returns -1.

I think I did it on purpose because this function is speed sensitive and
no user will change a valid rounding mode *constant* to a string that
is not ready.

So getting a double exception after deliberately sabotaging the module
seems pretty benign to me. :)


Consider the decimal part rejected.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue29190] Avoid possible errors in comparing strings

2017-01-07 Thread Serhiy Storchaka

Serhiy Storchaka added the comment:

In worst case ignoring an error can cause a mystical error later during 
executing an unrelated code or a crash in debug build. In both cases it is hard 
to find the location of the bug.

--
nosy: +haypo

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue29190] Avoid possible errors in comparing strings

2017-01-07 Thread Stefan Krah

Stefan Krah added the comment:

Also, if anyone changes the rounding-mode constants it is really their problem.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue29190] Avoid possible errors in comparing strings

2017-01-07 Thread Stefan Krah

Stefan Krah added the comment:

Quite honestly I prefer to do nothing. What is the worst that can happen?
A SystemError?

Not-ready unicode strings are an application bug.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue29190] Avoid possible errors in comparing strings

2017-01-07 Thread Xiang Zhang

Xiang Zhang added the comment:

Paste my point here:

I prefer checking the result PyUnicode_Compare to see it's a success or failure.
getrandom doesn't use any lower level unicode operations so it doesn't get a
responsibility to ready the unicode.

--
nosy: +xiang.zhang

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue29190] Avoid possible errors in comparing strings

2017-01-07 Thread Serhiy Storchaka

Serhiy Storchaka added the comment:

An alternative patch checks the result of PyUnicode_Compare() (as Xiang 
suggested) instead of checking the value before calling PyUnicode_Compare(). 
Stephan, what way do you prefer?

--
nosy: +facundobatista, mark.dickinson, rhettinger, skrah
Added file: http://bugs.python.org/file46186/unicode_compare_2.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue29190] Avoid possible errors in comparing strings

2017-01-06 Thread Serhiy Storchaka

New submission from Serhiy Storchaka:

PyUnicode_Compare() and PyUnicode_RichCompare() can raise an exception if one 
of arguments is not ready unicode object. The result is not always checked for 
error. Proposed patch gets rid of possible bugs. PyUnicode_Compare() and 
PyUnicode_RichCompare() in Modules/_pickle.c are replaced with 
_PyUnicode_EqualToASCIIString() and _PyUnicode_EqualToASCIIId() which never 
fail. Additional check is added in Modules/_decimal/_decimal.c to ensure that 
the string which is came from a user code is ready.

All other occurrences of PyUnicode_Compare() seems are called only with ready 
unicode objects.

--
components: Extension Modules
files: unicode_compare.patch
keywords: patch
messages: 284895
nosy: serhiy.storchaka
priority: normal
severity: normal
stage: patch review
status: open
title: Avoid possible errors in comparing strings
type: behavior
versions: Python 3.5, Python 3.6, Python 3.7
Added file: http://bugs.python.org/file46184/unicode_compare.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com