[jira] [Commented] (PROTON-2237) [python] Non-string message property keys not handled correctly
[ https://issues.apache.org/jira/browse/PROTON-2237?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17247451#comment-17247451 ] ASF subversion and git services commented on PROTON-2237: - Commit 0ef6be121f3f732a30d203aacb1b71e101ddb688 in qpid-proton's branch refs/heads/master from Kim van der Riet [ https://gitbox.apache.org/repos/asf?p=qpid-proton.git;h=0ef6be1 ] PROTON-2237: Minor fix: removed import of skipif in tests/proton_tests/message.py, not used. > [python] Non-string message property keys not handled correctly > --- > > Key: PROTON-2237 > URL: https://issues.apache.org/jira/browse/PROTON-2237 > Project: Qpid Proton > Issue Type: Bug > Components: python-binding >Reporter: Kim van der Riet >Assignee: Kim van der Riet >Priority: Major > > The AMQP 1.0 spec allows only string keys for message properties. > Proton's Python binding has a method (_message.py:91 _check_property_keys()) > for checking message property keys, but it does not handle all cases > correctly: > # Proton types Symbol and Char are derived from string, and are allowed in > the test. This results in an illegal encoding. > # Because in Python 2, many coders carelessly use string literals without > the required u'' prefix (and thus results in a bytes type), bytes types are > converted to unicode string types. However, the encode() function is being > used, which simply returns a binary type in Python 2 and raises an error in > Python 3. This should probably be the decode() method, which returns a string > and works for both Python 2 and 3. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[jira] [Commented] (PROTON-2237) [python] Non-string message property keys not handled correctly
[ https://issues.apache.org/jira/browse/PROTON-2237?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17245418#comment-17245418 ] ASF subversion and git services commented on PROTON-2237: - Commit 756d0de77637f3078e28b73fd24979b3e48e179a in qpid-proton's branch refs/heads/master from Kim van der Riet [ https://gitbox.apache.org/repos/asf?p=qpid-proton.git;h=756d0de ] Merge branch 'python-check-property-keys' Closes PR #256 / PROTON-2237 > [python] Non-string message property keys not handled correctly > --- > > Key: PROTON-2237 > URL: https://issues.apache.org/jira/browse/PROTON-2237 > Project: Qpid Proton > Issue Type: Bug > Components: python-binding >Reporter: Kim van der Riet >Assignee: Kim van der Riet >Priority: Major > > The AMQP 1.0 spec allows only string keys for message properties. > Proton's Python binding has a method (_message.py:91 _check_property_keys()) > for checking message property keys, but it does not handle all cases > correctly: > # Proton types Symbol and Char are derived from string, and are allowed in > the test. This results in an illegal encoding. > # Because in Python 2, many coders carelessly use string literals without > the required u'' prefix (and thus results in a bytes type), bytes types are > converted to unicode string types. However, the encode() function is being > used, which simply returns a binary type in Python 2 and raises an error in > Python 3. This should probably be the decode() method, which returns a string > and works for both Python 2 and 3. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[jira] [Commented] (PROTON-2237) [python] Non-string message property keys not handled correctly
[ https://issues.apache.org/jira/browse/PROTON-2237?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17245411#comment-17245411 ] ASF subversion and git services commented on PROTON-2237: - Commit 66f8696721fab2cc982c05eb7d94572bb2956841 in qpid-proton's branch refs/heads/master from Kim van der Riet [ https://gitbox.apache.org/repos/asf?p=qpid-proton.git;h=66f8696 ] PROTON-2237: Correct checking of Proton message property keys PROTON-2237: Alternative approach which converts all child classes of string/unicode to the base class, including proton symbol and char types. PROTON-2237: Changed logic of key check so that subclasses of string *except* proton.symbol and proton.char will be encoded as strings PROTON-2237: Added unit tests to check illegal key types are detected and handled, also subclasses of string type keys are converted to type string PROTON-2237: Finalized handling property keys, added tests for these cases. PROTON-2237: Fix for dictionary keys changed during iteration error, deeper test for key conversions PROTON-2237: Final tidy-up of logic and structure, added function doc to explain what is happening. NO-JIRA: Minor code format fix: added space into if stmt. PROTON-2237: Rearrange logic so as to avoid python version check. Minor re-arrange of tests. > [python] Non-string message property keys not handled correctly > --- > > Key: PROTON-2237 > URL: https://issues.apache.org/jira/browse/PROTON-2237 > Project: Qpid Proton > Issue Type: Bug > Components: python-binding >Reporter: Kim van der Riet >Assignee: Kim van der Riet >Priority: Major > > The AMQP 1.0 spec allows only string keys for message properties. > Proton's Python binding has a method (_message.py:91 _check_property_keys()) > for checking message property keys, but it does not handle all cases > correctly: > # Proton types Symbol and Char are derived from string, and are allowed in > the test. This results in an illegal encoding. > # Because in Python 2, many coders carelessly use string literals without > the required u'' prefix (and thus results in a bytes type), bytes types are > converted to unicode string types. However, the encode() function is being > used, which simply returns a binary type in Python 2 and raises an error in > Python 3. This should probably be the decode() method, which returns a string > and works for both Python 2 and 3. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[jira] [Commented] (PROTON-2237) [python] Non-string message property keys not handled correctly
[ https://issues.apache.org/jira/browse/PROTON-2237?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17245417#comment-17245417 ] ASF subversion and git services commented on PROTON-2237: - Commit 66f8696721fab2cc982c05eb7d94572bb2956841 in qpid-proton's branch refs/heads/master from Kim van der Riet [ https://gitbox.apache.org/repos/asf?p=qpid-proton.git;h=66f8696 ] PROTON-2237: Correct checking of Proton message property keys PROTON-2237: Alternative approach which converts all child classes of string/unicode to the base class, including proton symbol and char types. PROTON-2237: Changed logic of key check so that subclasses of string *except* proton.symbol and proton.char will be encoded as strings PROTON-2237: Added unit tests to check illegal key types are detected and handled, also subclasses of string type keys are converted to type string PROTON-2237: Finalized handling property keys, added tests for these cases. PROTON-2237: Fix for dictionary keys changed during iteration error, deeper test for key conversions PROTON-2237: Final tidy-up of logic and structure, added function doc to explain what is happening. NO-JIRA: Minor code format fix: added space into if stmt. PROTON-2237: Rearrange logic so as to avoid python version check. Minor re-arrange of tests. > [python] Non-string message property keys not handled correctly > --- > > Key: PROTON-2237 > URL: https://issues.apache.org/jira/browse/PROTON-2237 > Project: Qpid Proton > Issue Type: Bug > Components: python-binding >Reporter: Kim van der Riet >Assignee: Kim van der Riet >Priority: Major > > The AMQP 1.0 spec allows only string keys for message properties. > Proton's Python binding has a method (_message.py:91 _check_property_keys()) > for checking message property keys, but it does not handle all cases > correctly: > # Proton types Symbol and Char are derived from string, and are allowed in > the test. This results in an illegal encoding. > # Because in Python 2, many coders carelessly use string literals without > the required u'' prefix (and thus results in a bytes type), bytes types are > converted to unicode string types. However, the encode() function is being > used, which simply returns a binary type in Python 2 and raises an error in > Python 3. This should probably be the decode() method, which returns a string > and works for both Python 2 and 3. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[jira] [Commented] (PROTON-2237) [python] Non-string message property keys not handled correctly
[ https://issues.apache.org/jira/browse/PROTON-2237?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17245416#comment-17245416 ] ASF subversion and git services commented on PROTON-2237: - Commit 66f8696721fab2cc982c05eb7d94572bb2956841 in qpid-proton's branch refs/heads/master from Kim van der Riet [ https://gitbox.apache.org/repos/asf?p=qpid-proton.git;h=66f8696 ] PROTON-2237: Correct checking of Proton message property keys PROTON-2237: Alternative approach which converts all child classes of string/unicode to the base class, including proton symbol and char types. PROTON-2237: Changed logic of key check so that subclasses of string *except* proton.symbol and proton.char will be encoded as strings PROTON-2237: Added unit tests to check illegal key types are detected and handled, also subclasses of string type keys are converted to type string PROTON-2237: Finalized handling property keys, added tests for these cases. PROTON-2237: Fix for dictionary keys changed during iteration error, deeper test for key conversions PROTON-2237: Final tidy-up of logic and structure, added function doc to explain what is happening. NO-JIRA: Minor code format fix: added space into if stmt. PROTON-2237: Rearrange logic so as to avoid python version check. Minor re-arrange of tests. > [python] Non-string message property keys not handled correctly > --- > > Key: PROTON-2237 > URL: https://issues.apache.org/jira/browse/PROTON-2237 > Project: Qpid Proton > Issue Type: Bug > Components: python-binding >Reporter: Kim van der Riet >Assignee: Kim van der Riet >Priority: Major > > The AMQP 1.0 spec allows only string keys for message properties. > Proton's Python binding has a method (_message.py:91 _check_property_keys()) > for checking message property keys, but it does not handle all cases > correctly: > # Proton types Symbol and Char are derived from string, and are allowed in > the test. This results in an illegal encoding. > # Because in Python 2, many coders carelessly use string literals without > the required u'' prefix (and thus results in a bytes type), bytes types are > converted to unicode string types. However, the encode() function is being > used, which simply returns a binary type in Python 2 and raises an error in > Python 3. This should probably be the decode() method, which returns a string > and works for both Python 2 and 3. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[jira] [Commented] (PROTON-2237) [python] Non-string message property keys not handled correctly
[ https://issues.apache.org/jira/browse/PROTON-2237?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17245414#comment-17245414 ] ASF subversion and git services commented on PROTON-2237: - Commit 66f8696721fab2cc982c05eb7d94572bb2956841 in qpid-proton's branch refs/heads/master from Kim van der Riet [ https://gitbox.apache.org/repos/asf?p=qpid-proton.git;h=66f8696 ] PROTON-2237: Correct checking of Proton message property keys PROTON-2237: Alternative approach which converts all child classes of string/unicode to the base class, including proton symbol and char types. PROTON-2237: Changed logic of key check so that subclasses of string *except* proton.symbol and proton.char will be encoded as strings PROTON-2237: Added unit tests to check illegal key types are detected and handled, also subclasses of string type keys are converted to type string PROTON-2237: Finalized handling property keys, added tests for these cases. PROTON-2237: Fix for dictionary keys changed during iteration error, deeper test for key conversions PROTON-2237: Final tidy-up of logic and structure, added function doc to explain what is happening. NO-JIRA: Minor code format fix: added space into if stmt. PROTON-2237: Rearrange logic so as to avoid python version check. Minor re-arrange of tests. > [python] Non-string message property keys not handled correctly > --- > > Key: PROTON-2237 > URL: https://issues.apache.org/jira/browse/PROTON-2237 > Project: Qpid Proton > Issue Type: Bug > Components: python-binding >Reporter: Kim van der Riet >Assignee: Kim van der Riet >Priority: Major > > The AMQP 1.0 spec allows only string keys for message properties. > Proton's Python binding has a method (_message.py:91 _check_property_keys()) > for checking message property keys, but it does not handle all cases > correctly: > # Proton types Symbol and Char are derived from string, and are allowed in > the test. This results in an illegal encoding. > # Because in Python 2, many coders carelessly use string literals without > the required u'' prefix (and thus results in a bytes type), bytes types are > converted to unicode string types. However, the encode() function is being > used, which simply returns a binary type in Python 2 and raises an error in > Python 3. This should probably be the decode() method, which returns a string > and works for both Python 2 and 3. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[jira] [Commented] (PROTON-2237) [python] Non-string message property keys not handled correctly
[ https://issues.apache.org/jira/browse/PROTON-2237?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17245412#comment-17245412 ] ASF subversion and git services commented on PROTON-2237: - Commit 66f8696721fab2cc982c05eb7d94572bb2956841 in qpid-proton's branch refs/heads/master from Kim van der Riet [ https://gitbox.apache.org/repos/asf?p=qpid-proton.git;h=66f8696 ] PROTON-2237: Correct checking of Proton message property keys PROTON-2237: Alternative approach which converts all child classes of string/unicode to the base class, including proton symbol and char types. PROTON-2237: Changed logic of key check so that subclasses of string *except* proton.symbol and proton.char will be encoded as strings PROTON-2237: Added unit tests to check illegal key types are detected and handled, also subclasses of string type keys are converted to type string PROTON-2237: Finalized handling property keys, added tests for these cases. PROTON-2237: Fix for dictionary keys changed during iteration error, deeper test for key conversions PROTON-2237: Final tidy-up of logic and structure, added function doc to explain what is happening. NO-JIRA: Minor code format fix: added space into if stmt. PROTON-2237: Rearrange logic so as to avoid python version check. Minor re-arrange of tests. > [python] Non-string message property keys not handled correctly > --- > > Key: PROTON-2237 > URL: https://issues.apache.org/jira/browse/PROTON-2237 > Project: Qpid Proton > Issue Type: Bug > Components: python-binding >Reporter: Kim van der Riet >Assignee: Kim van der Riet >Priority: Major > > The AMQP 1.0 spec allows only string keys for message properties. > Proton's Python binding has a method (_message.py:91 _check_property_keys()) > for checking message property keys, but it does not handle all cases > correctly: > # Proton types Symbol and Char are derived from string, and are allowed in > the test. This results in an illegal encoding. > # Because in Python 2, many coders carelessly use string literals without > the required u'' prefix (and thus results in a bytes type), bytes types are > converted to unicode string types. However, the encode() function is being > used, which simply returns a binary type in Python 2 and raises an error in > Python 3. This should probably be the decode() method, which returns a string > and works for both Python 2 and 3. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[jira] [Commented] (PROTON-2237) [python] Non-string message property keys not handled correctly
[ https://issues.apache.org/jira/browse/PROTON-2237?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17245415#comment-17245415 ] ASF GitHub Bot commented on PROTON-2237: asfgit merged pull request #256: URL: https://github.com/apache/qpid-proton/pull/256 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > [python] Non-string message property keys not handled correctly > --- > > Key: PROTON-2237 > URL: https://issues.apache.org/jira/browse/PROTON-2237 > Project: Qpid Proton > Issue Type: Bug > Components: python-binding >Reporter: Kim van der Riet >Assignee: Kim van der Riet >Priority: Major > > The AMQP 1.0 spec allows only string keys for message properties. > Proton's Python binding has a method (_message.py:91 _check_property_keys()) > for checking message property keys, but it does not handle all cases > correctly: > # Proton types Symbol and Char are derived from string, and are allowed in > the test. This results in an illegal encoding. > # Because in Python 2, many coders carelessly use string literals without > the required u'' prefix (and thus results in a bytes type), bytes types are > converted to unicode string types. However, the encode() function is being > used, which simply returns a binary type in Python 2 and raises an error in > Python 3. This should probably be the decode() method, which returns a string > and works for both Python 2 and 3. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[jira] [Commented] (PROTON-2237) [python] Non-string message property keys not handled correctly
[ https://issues.apache.org/jira/browse/PROTON-2237?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17245413#comment-17245413 ] ASF subversion and git services commented on PROTON-2237: - Commit 66f8696721fab2cc982c05eb7d94572bb2956841 in qpid-proton's branch refs/heads/master from Kim van der Riet [ https://gitbox.apache.org/repos/asf?p=qpid-proton.git;h=66f8696 ] PROTON-2237: Correct checking of Proton message property keys PROTON-2237: Alternative approach which converts all child classes of string/unicode to the base class, including proton symbol and char types. PROTON-2237: Changed logic of key check so that subclasses of string *except* proton.symbol and proton.char will be encoded as strings PROTON-2237: Added unit tests to check illegal key types are detected and handled, also subclasses of string type keys are converted to type string PROTON-2237: Finalized handling property keys, added tests for these cases. PROTON-2237: Fix for dictionary keys changed during iteration error, deeper test for key conversions PROTON-2237: Final tidy-up of logic and structure, added function doc to explain what is happening. NO-JIRA: Minor code format fix: added space into if stmt. PROTON-2237: Rearrange logic so as to avoid python version check. Minor re-arrange of tests. > [python] Non-string message property keys not handled correctly > --- > > Key: PROTON-2237 > URL: https://issues.apache.org/jira/browse/PROTON-2237 > Project: Qpid Proton > Issue Type: Bug > Components: python-binding >Reporter: Kim van der Riet >Assignee: Kim van der Riet >Priority: Major > > The AMQP 1.0 spec allows only string keys for message properties. > Proton's Python binding has a method (_message.py:91 _check_property_keys()) > for checking message property keys, but it does not handle all cases > correctly: > # Proton types Symbol and Char are derived from string, and are allowed in > the test. This results in an illegal encoding. > # Because in Python 2, many coders carelessly use string literals without > the required u'' prefix (and thus results in a bytes type), bytes types are > converted to unicode string types. However, the encode() function is being > used, which simply returns a binary type in Python 2 and raises an error in > Python 3. This should probably be the decode() method, which returns a string > and works for both Python 2 and 3. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[jira] [Commented] (PROTON-2237) [python] Non-string message property keys not handled correctly
[ https://issues.apache.org/jira/browse/PROTON-2237?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17245410#comment-17245410 ] ASF subversion and git services commented on PROTON-2237: - Commit 66f8696721fab2cc982c05eb7d94572bb2956841 in qpid-proton's branch refs/heads/master from Kim van der Riet [ https://gitbox.apache.org/repos/asf?p=qpid-proton.git;h=66f8696 ] PROTON-2237: Correct checking of Proton message property keys PROTON-2237: Alternative approach which converts all child classes of string/unicode to the base class, including proton symbol and char types. PROTON-2237: Changed logic of key check so that subclasses of string *except* proton.symbol and proton.char will be encoded as strings PROTON-2237: Added unit tests to check illegal key types are detected and handled, also subclasses of string type keys are converted to type string PROTON-2237: Finalized handling property keys, added tests for these cases. PROTON-2237: Fix for dictionary keys changed during iteration error, deeper test for key conversions PROTON-2237: Final tidy-up of logic and structure, added function doc to explain what is happening. NO-JIRA: Minor code format fix: added space into if stmt. PROTON-2237: Rearrange logic so as to avoid python version check. Minor re-arrange of tests. > [python] Non-string message property keys not handled correctly > --- > > Key: PROTON-2237 > URL: https://issues.apache.org/jira/browse/PROTON-2237 > Project: Qpid Proton > Issue Type: Bug > Components: python-binding >Reporter: Kim van der Riet >Assignee: Kim van der Riet >Priority: Major > > The AMQP 1.0 spec allows only string keys for message properties. > Proton's Python binding has a method (_message.py:91 _check_property_keys()) > for checking message property keys, but it does not handle all cases > correctly: > # Proton types Symbol and Char are derived from string, and are allowed in > the test. This results in an illegal encoding. > # Because in Python 2, many coders carelessly use string literals without > the required u'' prefix (and thus results in a bytes type), bytes types are > converted to unicode string types. However, the encode() function is being > used, which simply returns a binary type in Python 2 and raises an error in > Python 3. This should probably be the decode() method, which returns a string > and works for both Python 2 and 3. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[jira] [Commented] (PROTON-2237) [python] Non-string message property keys not handled correctly
[ https://issues.apache.org/jira/browse/PROTON-2237?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17245409#comment-17245409 ] ASF subversion and git services commented on PROTON-2237: - Commit 66f8696721fab2cc982c05eb7d94572bb2956841 in qpid-proton's branch refs/heads/master from Kim van der Riet [ https://gitbox.apache.org/repos/asf?p=qpid-proton.git;h=66f8696 ] PROTON-2237: Correct checking of Proton message property keys PROTON-2237: Alternative approach which converts all child classes of string/unicode to the base class, including proton symbol and char types. PROTON-2237: Changed logic of key check so that subclasses of string *except* proton.symbol and proton.char will be encoded as strings PROTON-2237: Added unit tests to check illegal key types are detected and handled, also subclasses of string type keys are converted to type string PROTON-2237: Finalized handling property keys, added tests for these cases. PROTON-2237: Fix for dictionary keys changed during iteration error, deeper test for key conversions PROTON-2237: Final tidy-up of logic and structure, added function doc to explain what is happening. NO-JIRA: Minor code format fix: added space into if stmt. PROTON-2237: Rearrange logic so as to avoid python version check. Minor re-arrange of tests. > [python] Non-string message property keys not handled correctly > --- > > Key: PROTON-2237 > URL: https://issues.apache.org/jira/browse/PROTON-2237 > Project: Qpid Proton > Issue Type: Bug > Components: python-binding >Reporter: Kim van der Riet >Assignee: Kim van der Riet >Priority: Major > > The AMQP 1.0 spec allows only string keys for message properties. > Proton's Python binding has a method (_message.py:91 _check_property_keys()) > for checking message property keys, but it does not handle all cases > correctly: > # Proton types Symbol and Char are derived from string, and are allowed in > the test. This results in an illegal encoding. > # Because in Python 2, many coders carelessly use string literals without > the required u'' prefix (and thus results in a bytes type), bytes types are > converted to unicode string types. However, the encode() function is being > used, which simply returns a binary type in Python 2 and raises an error in > Python 3. This should probably be the decode() method, which returns a string > and works for both Python 2 and 3. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[jira] [Commented] (PROTON-2237) [python] Non-string message property keys not handled correctly
[ https://issues.apache.org/jira/browse/PROTON-2237?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17245407#comment-17245407 ] ASF subversion and git services commented on PROTON-2237: - Commit 66f8696721fab2cc982c05eb7d94572bb2956841 in qpid-proton's branch refs/heads/python-check-property-keys from Kim van der Riet [ https://gitbox.apache.org/repos/asf?p=qpid-proton.git;h=66f8696 ] PROTON-2237: Correct checking of Proton message property keys PROTON-2237: Alternative approach which converts all child classes of string/unicode to the base class, including proton symbol and char types. PROTON-2237: Changed logic of key check so that subclasses of string *except* proton.symbol and proton.char will be encoded as strings PROTON-2237: Added unit tests to check illegal key types are detected and handled, also subclasses of string type keys are converted to type string PROTON-2237: Finalized handling property keys, added tests for these cases. PROTON-2237: Fix for dictionary keys changed during iteration error, deeper test for key conversions PROTON-2237: Final tidy-up of logic and structure, added function doc to explain what is happening. NO-JIRA: Minor code format fix: added space into if stmt. PROTON-2237: Rearrange logic so as to avoid python version check. Minor re-arrange of tests. > [python] Non-string message property keys not handled correctly > --- > > Key: PROTON-2237 > URL: https://issues.apache.org/jira/browse/PROTON-2237 > Project: Qpid Proton > Issue Type: Bug > Components: python-binding >Reporter: Kim van der Riet >Assignee: Kim van der Riet >Priority: Major > > The AMQP 1.0 spec allows only string keys for message properties. > Proton's Python binding has a method (_message.py:91 _check_property_keys()) > for checking message property keys, but it does not handle all cases > correctly: > # Proton types Symbol and Char are derived from string, and are allowed in > the test. This results in an illegal encoding. > # Because in Python 2, many coders carelessly use string literals without > the required u'' prefix (and thus results in a bytes type), bytes types are > converted to unicode string types. However, the encode() function is being > used, which simply returns a binary type in Python 2 and raises an error in > Python 3. This should probably be the decode() method, which returns a string > and works for both Python 2 and 3. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[jira] [Commented] (PROTON-2237) [python] Non-string message property keys not handled correctly
[ https://issues.apache.org/jira/browse/PROTON-2237?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17245408#comment-17245408 ] ASF subversion and git services commented on PROTON-2237: - Commit 66f8696721fab2cc982c05eb7d94572bb2956841 in qpid-proton's branch refs/heads/python-check-property-keys from Kim van der Riet [ https://gitbox.apache.org/repos/asf?p=qpid-proton.git;h=66f8696 ] PROTON-2237: Correct checking of Proton message property keys PROTON-2237: Alternative approach which converts all child classes of string/unicode to the base class, including proton symbol and char types. PROTON-2237: Changed logic of key check so that subclasses of string *except* proton.symbol and proton.char will be encoded as strings PROTON-2237: Added unit tests to check illegal key types are detected and handled, also subclasses of string type keys are converted to type string PROTON-2237: Finalized handling property keys, added tests for these cases. PROTON-2237: Fix for dictionary keys changed during iteration error, deeper test for key conversions PROTON-2237: Final tidy-up of logic and structure, added function doc to explain what is happening. NO-JIRA: Minor code format fix: added space into if stmt. PROTON-2237: Rearrange logic so as to avoid python version check. Minor re-arrange of tests. > [python] Non-string message property keys not handled correctly > --- > > Key: PROTON-2237 > URL: https://issues.apache.org/jira/browse/PROTON-2237 > Project: Qpid Proton > Issue Type: Bug > Components: python-binding >Reporter: Kim van der Riet >Assignee: Kim van der Riet >Priority: Major > > The AMQP 1.0 spec allows only string keys for message properties. > Proton's Python binding has a method (_message.py:91 _check_property_keys()) > for checking message property keys, but it does not handle all cases > correctly: > # Proton types Symbol and Char are derived from string, and are allowed in > the test. This results in an illegal encoding. > # Because in Python 2, many coders carelessly use string literals without > the required u'' prefix (and thus results in a bytes type), bytes types are > converted to unicode string types. However, the encode() function is being > used, which simply returns a binary type in Python 2 and raises an error in > Python 3. This should probably be the decode() method, which returns a string > and works for both Python 2 and 3. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[jira] [Commented] (PROTON-2237) [python] Non-string message property keys not handled correctly
[ https://issues.apache.org/jira/browse/PROTON-2237?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17245402#comment-17245402 ] ASF subversion and git services commented on PROTON-2237: - Commit 66f8696721fab2cc982c05eb7d94572bb2956841 in qpid-proton's branch refs/heads/python-check-property-keys from Kim van der Riet [ https://gitbox.apache.org/repos/asf?p=qpid-proton.git;h=66f8696 ] PROTON-2237: Correct checking of Proton message property keys PROTON-2237: Alternative approach which converts all child classes of string/unicode to the base class, including proton symbol and char types. PROTON-2237: Changed logic of key check so that subclasses of string *except* proton.symbol and proton.char will be encoded as strings PROTON-2237: Added unit tests to check illegal key types are detected and handled, also subclasses of string type keys are converted to type string PROTON-2237: Finalized handling property keys, added tests for these cases. PROTON-2237: Fix for dictionary keys changed during iteration error, deeper test for key conversions PROTON-2237: Final tidy-up of logic and structure, added function doc to explain what is happening. NO-JIRA: Minor code format fix: added space into if stmt. PROTON-2237: Rearrange logic so as to avoid python version check. Minor re-arrange of tests. > [python] Non-string message property keys not handled correctly > --- > > Key: PROTON-2237 > URL: https://issues.apache.org/jira/browse/PROTON-2237 > Project: Qpid Proton > Issue Type: Bug > Components: python-binding >Reporter: Kim van der Riet >Assignee: Kim van der Riet >Priority: Major > > The AMQP 1.0 spec allows only string keys for message properties. > Proton's Python binding has a method (_message.py:91 _check_property_keys()) > for checking message property keys, but it does not handle all cases > correctly: > # Proton types Symbol and Char are derived from string, and are allowed in > the test. This results in an illegal encoding. > # Because in Python 2, many coders carelessly use string literals without > the required u'' prefix (and thus results in a bytes type), bytes types are > converted to unicode string types. However, the encode() function is being > used, which simply returns a binary type in Python 2 and raises an error in > Python 3. This should probably be the decode() method, which returns a string > and works for both Python 2 and 3. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[jira] [Commented] (PROTON-2237) [python] Non-string message property keys not handled correctly
[ https://issues.apache.org/jira/browse/PROTON-2237?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17245404#comment-17245404 ] ASF subversion and git services commented on PROTON-2237: - Commit 66f8696721fab2cc982c05eb7d94572bb2956841 in qpid-proton's branch refs/heads/python-check-property-keys from Kim van der Riet [ https://gitbox.apache.org/repos/asf?p=qpid-proton.git;h=66f8696 ] PROTON-2237: Correct checking of Proton message property keys PROTON-2237: Alternative approach which converts all child classes of string/unicode to the base class, including proton symbol and char types. PROTON-2237: Changed logic of key check so that subclasses of string *except* proton.symbol and proton.char will be encoded as strings PROTON-2237: Added unit tests to check illegal key types are detected and handled, also subclasses of string type keys are converted to type string PROTON-2237: Finalized handling property keys, added tests for these cases. PROTON-2237: Fix for dictionary keys changed during iteration error, deeper test for key conversions PROTON-2237: Final tidy-up of logic and structure, added function doc to explain what is happening. NO-JIRA: Minor code format fix: added space into if stmt. PROTON-2237: Rearrange logic so as to avoid python version check. Minor re-arrange of tests. > [python] Non-string message property keys not handled correctly > --- > > Key: PROTON-2237 > URL: https://issues.apache.org/jira/browse/PROTON-2237 > Project: Qpid Proton > Issue Type: Bug > Components: python-binding >Reporter: Kim van der Riet >Assignee: Kim van der Riet >Priority: Major > > The AMQP 1.0 spec allows only string keys for message properties. > Proton's Python binding has a method (_message.py:91 _check_property_keys()) > for checking message property keys, but it does not handle all cases > correctly: > # Proton types Symbol and Char are derived from string, and are allowed in > the test. This results in an illegal encoding. > # Because in Python 2, many coders carelessly use string literals without > the required u'' prefix (and thus results in a bytes type), bytes types are > converted to unicode string types. However, the encode() function is being > used, which simply returns a binary type in Python 2 and raises an error in > Python 3. This should probably be the decode() method, which returns a string > and works for both Python 2 and 3. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[jira] [Commented] (PROTON-2237) [python] Non-string message property keys not handled correctly
[ https://issues.apache.org/jira/browse/PROTON-2237?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17245406#comment-17245406 ] ASF subversion and git services commented on PROTON-2237: - Commit 66f8696721fab2cc982c05eb7d94572bb2956841 in qpid-proton's branch refs/heads/python-check-property-keys from Kim van der Riet [ https://gitbox.apache.org/repos/asf?p=qpid-proton.git;h=66f8696 ] PROTON-2237: Correct checking of Proton message property keys PROTON-2237: Alternative approach which converts all child classes of string/unicode to the base class, including proton symbol and char types. PROTON-2237: Changed logic of key check so that subclasses of string *except* proton.symbol and proton.char will be encoded as strings PROTON-2237: Added unit tests to check illegal key types are detected and handled, also subclasses of string type keys are converted to type string PROTON-2237: Finalized handling property keys, added tests for these cases. PROTON-2237: Fix for dictionary keys changed during iteration error, deeper test for key conversions PROTON-2237: Final tidy-up of logic and structure, added function doc to explain what is happening. NO-JIRA: Minor code format fix: added space into if stmt. PROTON-2237: Rearrange logic so as to avoid python version check. Minor re-arrange of tests. > [python] Non-string message property keys not handled correctly > --- > > Key: PROTON-2237 > URL: https://issues.apache.org/jira/browse/PROTON-2237 > Project: Qpid Proton > Issue Type: Bug > Components: python-binding >Reporter: Kim van der Riet >Assignee: Kim van der Riet >Priority: Major > > The AMQP 1.0 spec allows only string keys for message properties. > Proton's Python binding has a method (_message.py:91 _check_property_keys()) > for checking message property keys, but it does not handle all cases > correctly: > # Proton types Symbol and Char are derived from string, and are allowed in > the test. This results in an illegal encoding. > # Because in Python 2, many coders carelessly use string literals without > the required u'' prefix (and thus results in a bytes type), bytes types are > converted to unicode string types. However, the encode() function is being > used, which simply returns a binary type in Python 2 and raises an error in > Python 3. This should probably be the decode() method, which returns a string > and works for both Python 2 and 3. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[jira] [Commented] (PROTON-2237) [python] Non-string message property keys not handled correctly
[ https://issues.apache.org/jira/browse/PROTON-2237?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17245403#comment-17245403 ] ASF subversion and git services commented on PROTON-2237: - Commit 66f8696721fab2cc982c05eb7d94572bb2956841 in qpid-proton's branch refs/heads/python-check-property-keys from Kim van der Riet [ https://gitbox.apache.org/repos/asf?p=qpid-proton.git;h=66f8696 ] PROTON-2237: Correct checking of Proton message property keys PROTON-2237: Alternative approach which converts all child classes of string/unicode to the base class, including proton symbol and char types. PROTON-2237: Changed logic of key check so that subclasses of string *except* proton.symbol and proton.char will be encoded as strings PROTON-2237: Added unit tests to check illegal key types are detected and handled, also subclasses of string type keys are converted to type string PROTON-2237: Finalized handling property keys, added tests for these cases. PROTON-2237: Fix for dictionary keys changed during iteration error, deeper test for key conversions PROTON-2237: Final tidy-up of logic and structure, added function doc to explain what is happening. NO-JIRA: Minor code format fix: added space into if stmt. PROTON-2237: Rearrange logic so as to avoid python version check. Minor re-arrange of tests. > [python] Non-string message property keys not handled correctly > --- > > Key: PROTON-2237 > URL: https://issues.apache.org/jira/browse/PROTON-2237 > Project: Qpid Proton > Issue Type: Bug > Components: python-binding >Reporter: Kim van der Riet >Assignee: Kim van der Riet >Priority: Major > > The AMQP 1.0 spec allows only string keys for message properties. > Proton's Python binding has a method (_message.py:91 _check_property_keys()) > for checking message property keys, but it does not handle all cases > correctly: > # Proton types Symbol and Char are derived from string, and are allowed in > the test. This results in an illegal encoding. > # Because in Python 2, many coders carelessly use string literals without > the required u'' prefix (and thus results in a bytes type), bytes types are > converted to unicode string types. However, the encode() function is being > used, which simply returns a binary type in Python 2 and raises an error in > Python 3. This should probably be the decode() method, which returns a string > and works for both Python 2 and 3. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[jira] [Commented] (PROTON-2237) [python] Non-string message property keys not handled correctly
[ https://issues.apache.org/jira/browse/PROTON-2237?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17245405#comment-17245405 ] ASF subversion and git services commented on PROTON-2237: - Commit 66f8696721fab2cc982c05eb7d94572bb2956841 in qpid-proton's branch refs/heads/python-check-property-keys from Kim van der Riet [ https://gitbox.apache.org/repos/asf?p=qpid-proton.git;h=66f8696 ] PROTON-2237: Correct checking of Proton message property keys PROTON-2237: Alternative approach which converts all child classes of string/unicode to the base class, including proton symbol and char types. PROTON-2237: Changed logic of key check so that subclasses of string *except* proton.symbol and proton.char will be encoded as strings PROTON-2237: Added unit tests to check illegal key types are detected and handled, also subclasses of string type keys are converted to type string PROTON-2237: Finalized handling property keys, added tests for these cases. PROTON-2237: Fix for dictionary keys changed during iteration error, deeper test for key conversions PROTON-2237: Final tidy-up of logic and structure, added function doc to explain what is happening. NO-JIRA: Minor code format fix: added space into if stmt. PROTON-2237: Rearrange logic so as to avoid python version check. Minor re-arrange of tests. > [python] Non-string message property keys not handled correctly > --- > > Key: PROTON-2237 > URL: https://issues.apache.org/jira/browse/PROTON-2237 > Project: Qpid Proton > Issue Type: Bug > Components: python-binding >Reporter: Kim van der Riet >Assignee: Kim van der Riet >Priority: Major > > The AMQP 1.0 spec allows only string keys for message properties. > Proton's Python binding has a method (_message.py:91 _check_property_keys()) > for checking message property keys, but it does not handle all cases > correctly: > # Proton types Symbol and Char are derived from string, and are allowed in > the test. This results in an illegal encoding. > # Because in Python 2, many coders carelessly use string literals without > the required u'' prefix (and thus results in a bytes type), bytes types are > converted to unicode string types. However, the encode() function is being > used, which simply returns a binary type in Python 2 and raises an error in > Python 3. This should probably be the decode() method, which returns a string > and works for both Python 2 and 3. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[jira] [Commented] (PROTON-2237) [python] Non-string message property keys not handled correctly
[ https://issues.apache.org/jira/browse/PROTON-2237?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17245401#comment-17245401 ] ASF subversion and git services commented on PROTON-2237: - Commit 66f8696721fab2cc982c05eb7d94572bb2956841 in qpid-proton's branch refs/heads/python-check-property-keys from Kim van der Riet [ https://gitbox.apache.org/repos/asf?p=qpid-proton.git;h=66f8696 ] PROTON-2237: Correct checking of Proton message property keys PROTON-2237: Alternative approach which converts all child classes of string/unicode to the base class, including proton symbol and char types. PROTON-2237: Changed logic of key check so that subclasses of string *except* proton.symbol and proton.char will be encoded as strings PROTON-2237: Added unit tests to check illegal key types are detected and handled, also subclasses of string type keys are converted to type string PROTON-2237: Finalized handling property keys, added tests for these cases. PROTON-2237: Fix for dictionary keys changed during iteration error, deeper test for key conversions PROTON-2237: Final tidy-up of logic and structure, added function doc to explain what is happening. NO-JIRA: Minor code format fix: added space into if stmt. PROTON-2237: Rearrange logic so as to avoid python version check. Minor re-arrange of tests. > [python] Non-string message property keys not handled correctly > --- > > Key: PROTON-2237 > URL: https://issues.apache.org/jira/browse/PROTON-2237 > Project: Qpid Proton > Issue Type: Bug > Components: python-binding >Reporter: Kim van der Riet >Assignee: Kim van der Riet >Priority: Major > > The AMQP 1.0 spec allows only string keys for message properties. > Proton's Python binding has a method (_message.py:91 _check_property_keys()) > for checking message property keys, but it does not handle all cases > correctly: > # Proton types Symbol and Char are derived from string, and are allowed in > the test. This results in an illegal encoding. > # Because in Python 2, many coders carelessly use string literals without > the required u'' prefix (and thus results in a bytes type), bytes types are > converted to unicode string types. However, the encode() function is being > used, which simply returns a binary type in Python 2 and raises an error in > Python 3. This should probably be the decode() method, which returns a string > and works for both Python 2 and 3. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[jira] [Commented] (PROTON-2237) [python] Non-string message property keys not handled correctly
[ https://issues.apache.org/jira/browse/PROTON-2237?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17245373#comment-17245373 ] ASF GitHub Bot commented on PROTON-2237: kpvdr opened a new pull request #256: URL: https://github.com/apache/qpid-proton/pull/256 Proposed fix for PROTON-2237. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > [python] Non-string message property keys not handled correctly > --- > > Key: PROTON-2237 > URL: https://issues.apache.org/jira/browse/PROTON-2237 > Project: Qpid Proton > Issue Type: Bug > Components: python-binding >Reporter: Kim van der Riet >Assignee: Kim van der Riet >Priority: Major > > The AMQP 1.0 spec allows only string keys for message properties. > Proton's Python binding has a method (_message.py:91 _check_property_keys()) > for checking message property keys, but it does not handle all cases > correctly: > # Proton types Symbol and Char are derived from string, and are allowed in > the test. This results in an illegal encoding. > # Because in Python 2, many coders carelessly use string literals without > the required u'' prefix (and thus results in a bytes type), bytes types are > converted to unicode string types. However, the encode() function is being > used, which simply returns a binary type in Python 2 and raises an error in > Python 3. This should probably be the decode() method, which returns a string > and works for both Python 2 and 3. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[jira] [Commented] (PROTON-2237) [python] Non-string message property keys not handled correctly
[ https://issues.apache.org/jira/browse/PROTON-2237?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17245367#comment-17245367 ] ASF GitHub Bot commented on PROTON-2237: kpvdr closed pull request #256: URL: https://github.com/apache/qpid-proton/pull/256 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > [python] Non-string message property keys not handled correctly > --- > > Key: PROTON-2237 > URL: https://issues.apache.org/jira/browse/PROTON-2237 > Project: Qpid Proton > Issue Type: Bug > Components: python-binding >Reporter: Kim van der Riet >Assignee: Kim van der Riet >Priority: Major > > The AMQP 1.0 spec allows only string keys for message properties. > Proton's Python binding has a method (_message.py:91 _check_property_keys()) > for checking message property keys, but it does not handle all cases > correctly: > # Proton types Symbol and Char are derived from string, and are allowed in > the test. This results in an illegal encoding. > # Because in Python 2, many coders carelessly use string literals without > the required u'' prefix (and thus results in a bytes type), bytes types are > converted to unicode string types. However, the encode() function is being > used, which simply returns a binary type in Python 2 and raises an error in > Python 3. This should probably be the decode() method, which returns a string > and works for both Python 2 and 3. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[jira] [Commented] (PROTON-2237) [python] Non-string message property keys not handled correctly
[ https://issues.apache.org/jira/browse/PROTON-2237?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17244272#comment-17244272 ] ASF GitHub Bot commented on PROTON-2237: ssorj commented on pull request #256: URL: https://github.com/apache/qpid-proton/pull/256#issuecomment-738999720 @kpvdr would you close this one if it's ready? Note that there is a comment notation that will close PRs for you. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > [python] Non-string message property keys not handled correctly > --- > > Key: PROTON-2237 > URL: https://issues.apache.org/jira/browse/PROTON-2237 > Project: Qpid Proton > Issue Type: Bug > Components: python-binding >Reporter: Kim van der Riet >Priority: Major > > The AMQP 1.0 spec allows only string keys for message properties. > Proton's Python binding has a method (_message.py:91 _check_property_keys()) > for checking message property keys, but it does not handle all cases > correctly: > # Proton types Symbol and Char are derived from string, and are allowed in > the test. This results in an illegal encoding. > # Because in Python 2, many coders carelessly use string literals without > the required u'' prefix (and thus results in a bytes type), bytes types are > converted to unicode string types. However, the encode() function is being > used, which simply returns a binary type in Python 2 and raises an error in > Python 3. This should probably be the decode() method, which returns a string > and works for both Python 2 and 3. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[jira] [Commented] (PROTON-2237) [python] Non-string message property keys not handled correctly
[ https://issues.apache.org/jira/browse/PROTON-2237?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17179687#comment-17179687 ] ASF subversion and git services commented on PROTON-2237: - Commit 877d2dfff5f2fb0ed75f82fe88c61d2f589b70c2 in qpid-proton's branch refs/heads/python-check-property-keys from Kim van der Riet [ https://gitbox.apache.org/repos/asf?p=qpid-proton.git;h=877d2df ] PROTON-2237: Rearrange logic so as to avoid python version check. Minor re-arrange of tests. > [python] Non-string message property keys not handled correctly > --- > > Key: PROTON-2237 > URL: https://issues.apache.org/jira/browse/PROTON-2237 > Project: Qpid Proton > Issue Type: Bug > Components: python-binding >Reporter: Kim van der Riet >Priority: Major > > The AMQP 1.0 spec allows only string keys for message properties. > Proton's Python binding has a method (_message.py:91 _check_property_keys()) > for checking message property keys, but it does not handle all cases > correctly: > # Proton types Symbol and Char are derived from string, and are allowed in > the test. This results in an illegal encoding. > # Because in Python 2, many coders carelessly use string literals without > the required u'' prefix (and thus results in a bytes type), bytes types are > converted to unicode string types. However, the encode() function is being > used, which simply returns a binary type in Python 2 and raises an error in > Python 3. This should probably be the decode() method, which returns a string > and works for both Python 2 and 3. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[jira] [Commented] (PROTON-2237) [python] Non-string message property keys not handled correctly
[
https://issues.apache.org/jira/browse/PROTON-2237?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17176626#comment-17176626
]
ASF GitHub Bot commented on PROTON-2237:
kpvdr commented on a change in pull request #256:
URL: https://github.com/apache/qpid-proton/pull/256#discussion_r469565080
##
File path: python/proton/_message.py
##
@@ -90,13 +90,18 @@ def _check(self, err):
def _check_property_keys(self):
for k in self.properties.keys():
-if isinstance(k, unicode):
-# py2 unicode, py3 str (via hack definition)
+# Check for string types. (py2: unicode, py3: str via type hack
above)
+# or string subclasses. Exclude string subclasses symbol and char.
+if isinstance(k, unicode) and not (type(k) is symbol or type(k) is
char):
+# Convert string subclasses to string
+if not type(k) is unicode:
+self.properties[unicode(k)] = self.properties.pop(k)
continue
-# If key is binary then change to string
-elif isinstance(k, str):
-# py2 str
-self.properties[k.encode('utf-8')] = self.properties.pop(k)
+# If key is binary then change to string. Exclude bytes subclass
decimal128.
+# Mostly for py2 users who encode strings without using the u''
prefix
+# but py3 bytes() type will be converted also
+elif isinstance(k, bytes) and not (type(k) is decimal128):
Review comment:
It does not work because the assumption that py3 strings are already
covered is not valid. If the key is a symbol or char, they are excluded from
the above case, and they will pass isinstance(k, str) and be converted here,
resulting in an error.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> [python] Non-string message property keys not handled correctly
> ---
>
> Key: PROTON-2237
> URL: https://issues.apache.org/jira/browse/PROTON-2237
> Project: Qpid Proton
> Issue Type: Bug
> Components: python-binding
>Reporter: Kim van der Riet
>Priority: Major
>
> The AMQP 1.0 spec allows only string keys for message properties.
> Proton's Python binding has a method (_message.py:91 _check_property_keys())
> for checking message property keys, but it does not handle all cases
> correctly:
> # Proton types Symbol and Char are derived from string, and are allowed in
> the test. This results in an illegal encoding.
> # Because in Python 2, many coders carelessly use string literals without
> the required u'' prefix (and thus results in a bytes type), bytes types are
> converted to unicode string types. However, the encode() function is being
> used, which simply returns a binary type in Python 2 and raises an error in
> Python 3. This should probably be the decode() method, which returns a string
> and works for both Python 2 and 3.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org
[jira] [Commented] (PROTON-2237) [python] Non-string message property keys not handled correctly
[
https://issues.apache.org/jira/browse/PROTON-2237?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17176620#comment-17176620
]
ASF GitHub Bot commented on PROTON-2237:
kpvdr commented on a change in pull request #256:
URL: https://github.com/apache/qpid-proton/pull/256#discussion_r469565080
##
File path: python/proton/_message.py
##
@@ -90,13 +90,18 @@ def _check(self, err):
def _check_property_keys(self):
for k in self.properties.keys():
-if isinstance(k, unicode):
-# py2 unicode, py3 str (via hack definition)
+# Check for string types. (py2: unicode, py3: str via type hack
above)
+# or string subclasses. Exclude string subclasses symbol and char.
+if isinstance(k, unicode) and not (type(k) is symbol or type(k) is
char):
+# Convert string subclasses to string
+if not type(k) is unicode:
+self.properties[unicode(k)] = self.properties.pop(k)
continue
-# If key is binary then change to string
-elif isinstance(k, str):
-# py2 str
-self.properties[k.encode('utf-8')] = self.properties.pop(k)
+# If key is binary then change to string. Exclude bytes subclass
decimal128.
+# Mostly for py2 users who encode strings without using the u''
prefix
+# but py3 bytes() type will be converted also
+elif isinstance(k, bytes) and not (type(k) is decimal128):
Review comment:
Good point. Agreed.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> [python] Non-string message property keys not handled correctly
> ---
>
> Key: PROTON-2237
> URL: https://issues.apache.org/jira/browse/PROTON-2237
> Project: Qpid Proton
> Issue Type: Bug
> Components: python-binding
>Reporter: Kim van der Riet
>Priority: Major
>
> The AMQP 1.0 spec allows only string keys for message properties.
> Proton's Python binding has a method (_message.py:91 _check_property_keys())
> for checking message property keys, but it does not handle all cases
> correctly:
> # Proton types Symbol and Char are derived from string, and are allowed in
> the test. This results in an illegal encoding.
> # Because in Python 2, many coders carelessly use string literals without
> the required u'' prefix (and thus results in a bytes type), bytes types are
> converted to unicode string types. However, the encode() function is being
> used, which simply returns a binary type in Python 2 and raises an error in
> Python 3. This should probably be the decode() method, which returns a string
> and works for both Python 2 and 3.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org
[jira] [Commented] (PROTON-2237) [python] Non-string message property keys not handled correctly
[ https://issues.apache.org/jira/browse/PROTON-2237?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17176618#comment-17176618 ] ASF GitHub Bot commented on PROTON-2237: kpvdr commented on a change in pull request #256: URL: https://github.com/apache/qpid-proton/pull/256#discussion_r469561876 ## File path: python/proton/_message.py ## @@ -90,13 +90,18 @@ def _check(self, err): def _check_property_keys(self): for k in self.properties.keys(): -if isinstance(k, unicode): -# py2 unicode, py3 str (via hack definition) +# Check for string types. (py2: unicode, py3: str via type hack above) +# or string subclasses. Exclude string subclasses symbol and char. +if isinstance(k, unicode) and not (type(k) is symbol or type(k) is char): +# Convert string subclasses to string +if not type(k) is unicode: +self.properties[unicode(k)] = self.properties.pop(k) Review comment: No, encode won't work directly on subclasses of unicode. Data.put_mappings throws a KeyError, as the subclass name is not known. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > [python] Non-string message property keys not handled correctly > --- > > Key: PROTON-2237 > URL: https://issues.apache.org/jira/browse/PROTON-2237 > Project: Qpid Proton > Issue Type: Bug > Components: python-binding >Reporter: Kim van der Riet >Priority: Major > > The AMQP 1.0 spec allows only string keys for message properties. > Proton's Python binding has a method (_message.py:91 _check_property_keys()) > for checking message property keys, but it does not handle all cases > correctly: > # Proton types Symbol and Char are derived from string, and are allowed in > the test. This results in an illegal encoding. > # Because in Python 2, many coders carelessly use string literals without > the required u'' prefix (and thus results in a bytes type), bytes types are > converted to unicode string types. However, the encode() function is being > used, which simply returns a binary type in Python 2 and raises an error in > Python 3. This should probably be the decode() method, which returns a string > and works for both Python 2 and 3. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[jira] [Commented] (PROTON-2237) [python] Non-string message property keys not handled correctly
[ https://issues.apache.org/jira/browse/PROTON-2237?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17176616#comment-17176616 ] ASF GitHub Bot commented on PROTON-2237: kpvdr commented on pull request #256: URL: https://github.com/apache/qpid-proton/pull/256#issuecomment-673121713 This should be close now. Passes both Py2 and Py3 tests. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > [python] Non-string message property keys not handled correctly > --- > > Key: PROTON-2237 > URL: https://issues.apache.org/jira/browse/PROTON-2237 > Project: Qpid Proton > Issue Type: Bug > Components: python-binding >Reporter: Kim van der Riet >Priority: Major > > The AMQP 1.0 spec allows only string keys for message properties. > Proton's Python binding has a method (_message.py:91 _check_property_keys()) > for checking message property keys, but it does not handle all cases > correctly: > # Proton types Symbol and Char are derived from string, and are allowed in > the test. This results in an illegal encoding. > # Because in Python 2, many coders carelessly use string literals without > the required u'' prefix (and thus results in a bytes type), bytes types are > converted to unicode string types. However, the encode() function is being > used, which simply returns a binary type in Python 2 and raises an error in > Python 3. This should probably be the decode() method, which returns a string > and works for both Python 2 and 3. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[jira] [Commented] (PROTON-2237) [python] Non-string message property keys not handled correctly
[ https://issues.apache.org/jira/browse/PROTON-2237?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17176614#comment-17176614 ] ASF subversion and git services commented on PROTON-2237: - Commit e089561d0b560a12198f29211aba95fa758fa81d in qpid-proton's branch refs/heads/python-check-property-keys from Kim van der Riet [ https://gitbox.apache.org/repos/asf?p=qpid-proton.git;h=e089561 ] PROTON-2237: Final tidy-up of logic and structure, added function doc to explain what is happening. > [python] Non-string message property keys not handled correctly > --- > > Key: PROTON-2237 > URL: https://issues.apache.org/jira/browse/PROTON-2237 > Project: Qpid Proton > Issue Type: Bug > Components: python-binding >Reporter: Kim van der Riet >Priority: Major > > The AMQP 1.0 spec allows only string keys for message properties. > Proton's Python binding has a method (_message.py:91 _check_property_keys()) > for checking message property keys, but it does not handle all cases > correctly: > # Proton types Symbol and Char are derived from string, and are allowed in > the test. This results in an illegal encoding. > # Because in Python 2, many coders carelessly use string literals without > the required u'' prefix (and thus results in a bytes type), bytes types are > converted to unicode string types. However, the encode() function is being > used, which simply returns a binary type in Python 2 and raises an error in > Python 3. This should probably be the decode() method, which returns a string > and works for both Python 2 and 3. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[jira] [Commented] (PROTON-2237) [python] Non-string message property keys not handled correctly
[ https://issues.apache.org/jira/browse/PROTON-2237?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17176579#comment-17176579 ] ASF GitHub Bot commented on PROTON-2237: kpvdr commented on a change in pull request #256: URL: https://github.com/apache/qpid-proton/pull/256#discussion_r469511394 ## File path: python/proton/_message.py ## @@ -90,13 +90,18 @@ def _check(self, err): def _check_property_keys(self): for k in self.properties.keys(): -if isinstance(k, unicode): -# py2 unicode, py3 str (via hack definition) +# Check for string types. (py2: unicode, py3: str via type hack above) +# or string subclasses. Exclude string subclasses symbol and char. +if isinstance(k, unicode) and not (type(k) is symbol or type(k) is char): +# Convert string subclasses to string +if not type(k) is unicode: +self.properties[unicode(k)] = self.properties.pop(k) continue Review comment: OK, you are ahead of me. Given the if / elif / else structure, removing 'continue' would be logically the same, I think. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > [python] Non-string message property keys not handled correctly > --- > > Key: PROTON-2237 > URL: https://issues.apache.org/jira/browse/PROTON-2237 > Project: Qpid Proton > Issue Type: Bug > Components: python-binding >Reporter: Kim van der Riet >Priority: Major > > The AMQP 1.0 spec allows only string keys for message properties. > Proton's Python binding has a method (_message.py:91 _check_property_keys()) > for checking message property keys, but it does not handle all cases > correctly: > # Proton types Symbol and Char are derived from string, and are allowed in > the test. This results in an illegal encoding. > # Because in Python 2, many coders carelessly use string literals without > the required u'' prefix (and thus results in a bytes type), bytes types are > converted to unicode string types. However, the encode() function is being > used, which simply returns a binary type in Python 2 and raises an error in > Python 3. This should probably be the decode() method, which returns a string > and works for both Python 2 and 3. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[jira] [Commented] (PROTON-2237) [python] Non-string message property keys not handled correctly
[ https://issues.apache.org/jira/browse/PROTON-2237?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17176577#comment-17176577 ] ASF GitHub Bot commented on PROTON-2237: kpvdr commented on a change in pull request #256: URL: https://github.com/apache/qpid-proton/pull/256#discussion_r469510090 ## File path: python/proton/_message.py ## @@ -90,13 +90,18 @@ def _check(self, err): def _check_property_keys(self): for k in self.properties.keys(): -if isinstance(k, unicode): -# py2 unicode, py3 str (via hack definition) +# Check for string types. (py2: unicode, py3: str via type hack above) +# or string subclasses. Exclude string subclasses symbol and char. +if isinstance(k, unicode) and not (type(k) is symbol or type(k) is char): +# Convert string subclasses to string +if not type(k) is unicode: +self.properties[unicode(k)] = self.properties.pop(k) continue Review comment: I think it is a way of jumping out of the code within the for loop and continuing with the loop itself: ``` for k in self.property.keys(): if : if : continue other stuff... ``` is a way of avoiding other stuff if is true. But I am sure there is a way of rearranging this to avoid it. Certainly it is not possible to replace continue with pass, that is not logically the same, as other stuff would also then be executed. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > [python] Non-string message property keys not handled correctly > --- > > Key: PROTON-2237 > URL: https://issues.apache.org/jira/browse/PROTON-2237 > Project: Qpid Proton > Issue Type: Bug > Components: python-binding >Reporter: Kim van der Riet >Priority: Major > > The AMQP 1.0 spec allows only string keys for message properties. > Proton's Python binding has a method (_message.py:91 _check_property_keys()) > for checking message property keys, but it does not handle all cases > correctly: > # Proton types Symbol and Char are derived from string, and are allowed in > the test. This results in an illegal encoding. > # Because in Python 2, many coders carelessly use string literals without > the required u'' prefix (and thus results in a bytes type), bytes types are > converted to unicode string types. However, the encode() function is being > used, which simply returns a binary type in Python 2 and raises an error in > Python 3. This should probably be the decode() method, which returns a string > and works for both Python 2 and 3. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[jira] [Commented] (PROTON-2237) [python] Non-string message property keys not handled correctly
[ https://issues.apache.org/jira/browse/PROTON-2237?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17176576#comment-17176576 ] ASF GitHub Bot commented on PROTON-2237: kpvdr commented on a change in pull request #256: URL: https://github.com/apache/qpid-proton/pull/256#discussion_r469510090 ## File path: python/proton/_message.py ## @@ -90,13 +90,18 @@ def _check(self, err): def _check_property_keys(self): for k in self.properties.keys(): -if isinstance(k, unicode): -# py2 unicode, py3 str (via hack definition) +# Check for string types. (py2: unicode, py3: str via type hack above) +# or string subclasses. Exclude string subclasses symbol and char. +if isinstance(k, unicode) and not (type(k) is symbol or type(k) is char): +# Convert string subclasses to string +if not type(k) is unicode: +self.properties[unicode(k)] = self.properties.pop(k) continue Review comment: I think it is a way of jumping out of the code within the for loop and continuing with the loop itself: ``` for k in self.property.keys(): if : if : continue other stuff... ``` is a way of avoiding other stuff if xxx is true. But I am sure there is a way of rearranging this to avoid it. Certainly it is not possible to replace continue with pass, that is not logically the same, as other stuff would also then be executed. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > [python] Non-string message property keys not handled correctly > --- > > Key: PROTON-2237 > URL: https://issues.apache.org/jira/browse/PROTON-2237 > Project: Qpid Proton > Issue Type: Bug > Components: python-binding >Reporter: Kim van der Riet >Priority: Major > > The AMQP 1.0 spec allows only string keys for message properties. > Proton's Python binding has a method (_message.py:91 _check_property_keys()) > for checking message property keys, but it does not handle all cases > correctly: > # Proton types Symbol and Char are derived from string, and are allowed in > the test. This results in an illegal encoding. > # Because in Python 2, many coders carelessly use string literals without > the required u'' prefix (and thus results in a bytes type), bytes types are > converted to unicode string types. However, the encode() function is being > used, which simply returns a binary type in Python 2 and raises an error in > Python 3. This should probably be the decode() method, which returns a string > and works for both Python 2 and 3. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[jira] [Commented] (PROTON-2237) [python] Non-string message property keys not handled correctly
[ https://issues.apache.org/jira/browse/PROTON-2237?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17176569#comment-17176569 ] ASF GitHub Bot commented on PROTON-2237: kpvdr commented on a change in pull request #256: URL: https://github.com/apache/qpid-proton/pull/256#discussion_r469505961 ## File path: python/proton/_message.py ## @@ -90,13 +90,18 @@ def _check(self, err): def _check_property_keys(self): Review comment: Agreed. Some text would be helpful. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > [python] Non-string message property keys not handled correctly > --- > > Key: PROTON-2237 > URL: https://issues.apache.org/jira/browse/PROTON-2237 > Project: Qpid Proton > Issue Type: Bug > Components: python-binding >Reporter: Kim van der Riet >Priority: Major > > The AMQP 1.0 spec allows only string keys for message properties. > Proton's Python binding has a method (_message.py:91 _check_property_keys()) > for checking message property keys, but it does not handle all cases > correctly: > # Proton types Symbol and Char are derived from string, and are allowed in > the test. This results in an illegal encoding. > # Because in Python 2, many coders carelessly use string literals without > the required u'' prefix (and thus results in a bytes type), bytes types are > converted to unicode string types. However, the encode() function is being > used, which simply returns a binary type in Python 2 and raises an error in > Python 3. This should probably be the decode() method, which returns a string > and works for both Python 2 and 3. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[jira] [Commented] (PROTON-2237) [python] Non-string message property keys not handled correctly
[
https://issues.apache.org/jira/browse/PROTON-2237?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17176567#comment-17176567
]
ASF GitHub Bot commented on PROTON-2237:
kpvdr commented on a change in pull request #256:
URL: https://github.com/apache/qpid-proton/pull/256#discussion_r469505140
##
File path: python/proton/_message.py
##
@@ -90,13 +90,18 @@ def _check(self, err):
def _check_property_keys(self):
for k in self.properties.keys():
-if isinstance(k, unicode):
-# py2 unicode, py3 str (via hack definition)
+# Check for string types. (py2: unicode, py3: str via type hack
above)
+# or string subclasses. Exclude string subclasses symbol and char.
+if isinstance(k, unicode) and not (type(k) is symbol or type(k) is
char):
+# Convert string subclasses to string
+if not type(k) is unicode:
+self.properties[unicode(k)] = self.properties.pop(k)
continue
-# If key is binary then change to string
-elif isinstance(k, str):
-# py2 str
-self.properties[k.encode('utf-8')] = self.properties.pop(k)
+# If key is binary then change to string. Exclude bytes subclass
decimal128.
+# Mostly for py2 users who encode strings without using the u''
prefix
+# but py3 bytes() type will be converted also
Review comment:
I have thought of this, and it is a valid point. It was only for Py2
users which did not use the u'' prefix that this conversion was included. For
Py3 users, this could and probably should be an error.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> [python] Non-string message property keys not handled correctly
> ---
>
> Key: PROTON-2237
> URL: https://issues.apache.org/jira/browse/PROTON-2237
> Project: Qpid Proton
> Issue Type: Bug
> Components: python-binding
>Reporter: Kim van der Riet
>Priority: Major
>
> The AMQP 1.0 spec allows only string keys for message properties.
> Proton's Python binding has a method (_message.py:91 _check_property_keys())
> for checking message property keys, but it does not handle all cases
> correctly:
> # Proton types Symbol and Char are derived from string, and are allowed in
> the test. This results in an illegal encoding.
> # Because in Python 2, many coders carelessly use string literals without
> the required u'' prefix (and thus results in a bytes type), bytes types are
> converted to unicode string types. However, the encode() function is being
> used, which simply returns a binary type in Python 2 and raises an error in
> Python 3. This should probably be the decode() method, which returns a string
> and works for both Python 2 and 3.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org
[jira] [Commented] (PROTON-2237) [python] Non-string message property keys not handled correctly
[ https://issues.apache.org/jira/browse/PROTON-2237?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17176565#comment-17176565 ] ASF GitHub Bot commented on PROTON-2237: kpvdr commented on a change in pull request #256: URL: https://github.com/apache/qpid-proton/pull/256#discussion_r469504092 ## File path: python/proton/_message.py ## @@ -90,13 +90,18 @@ def _check(self, err): def _check_property_keys(self): for k in self.properties.keys(): -if isinstance(k, unicode): -# py2 unicode, py3 str (via hack definition) +# Check for string types. (py2: unicode, py3: str via type hack above) +# or string subclasses. Exclude string subclasses symbol and char. +if isinstance(k, unicode) and not (type(k) is symbol or type(k) is char): +# Convert string subclasses to string +if not type(k) is unicode: +self.properties[unicode(k)] = self.properties.pop(k) Review comment: This is to make the change only if the key is a subclass of unicode, but not unicode itself. As we first check with isinstance(unicode), all unicode and their subclasses are included, and continue will be called. But for sublcasses only, a conversion is made, hence the second test using type() is unicode. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > [python] Non-string message property keys not handled correctly > --- > > Key: PROTON-2237 > URL: https://issues.apache.org/jira/browse/PROTON-2237 > Project: Qpid Proton > Issue Type: Bug > Components: python-binding >Reporter: Kim van der Riet >Priority: Major > > The AMQP 1.0 spec allows only string keys for message properties. > Proton's Python binding has a method (_message.py:91 _check_property_keys()) > for checking message property keys, but it does not handle all cases > correctly: > # Proton types Symbol and Char are derived from string, and are allowed in > the test. This results in an illegal encoding. > # Because in Python 2, many coders carelessly use string literals without > the required u'' prefix (and thus results in a bytes type), bytes types are > converted to unicode string types. However, the encode() function is being > used, which simply returns a binary type in Python 2 and raises an error in > Python 3. This should probably be the decode() method, which returns a string > and works for both Python 2 and 3. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[jira] [Commented] (PROTON-2237) [python] Non-string message property keys not handled correctly
[ https://issues.apache.org/jira/browse/PROTON-2237?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17176560#comment-17176560 ] ASF subversion and git services commented on PROTON-2237: - Commit 3b916e1213ec865a7056c7dc38dd85c1dc36cb60 in qpid-proton's branch refs/heads/python-check-property-keys from Kim van der Riet [ https://gitbox.apache.org/repos/asf?p=qpid-proton.git;h=3b916e1 ] PROTON-2237: Fix for dictionary keys changed during iteration error, deeper test for key conversions > [python] Non-string message property keys not handled correctly > --- > > Key: PROTON-2237 > URL: https://issues.apache.org/jira/browse/PROTON-2237 > Project: Qpid Proton > Issue Type: Bug > Components: python-binding >Reporter: Kim van der Riet >Priority: Major > > The AMQP 1.0 spec allows only string keys for message properties. > Proton's Python binding has a method (_message.py:91 _check_property_keys()) > for checking message property keys, but it does not handle all cases > correctly: > # Proton types Symbol and Char are derived from string, and are allowed in > the test. This results in an illegal encoding. > # Because in Python 2, many coders carelessly use string literals without > the required u'' prefix (and thus results in a bytes type), bytes types are > converted to unicode string types. However, the encode() function is being > used, which simply returns a binary type in Python 2 and raises an error in > Python 3. This should probably be the decode() method, which returns a string > and works for both Python 2 and 3. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[jira] [Commented] (PROTON-2237) [python] Non-string message property keys not handled correctly
[
https://issues.apache.org/jira/browse/PROTON-2237?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17176545#comment-17176545
]
ASF GitHub Bot commented on PROTON-2237:
astitcher commented on a change in pull request #256:
URL: https://github.com/apache/qpid-proton/pull/256#discussion_r469470459
##
File path: python/proton/_message.py
##
@@ -90,13 +90,18 @@ def _check(self, err):
def _check_property_keys(self):
Review comment:
It would be really helpful here to explain what the AMQP limitations on
property keys are. I can't figure out from reading this code what the actual
point of the code is.
##
File path: python/proton/_message.py
##
@@ -90,13 +90,18 @@ def _check(self, err):
def _check_property_keys(self):
for k in self.properties.keys():
-if isinstance(k, unicode):
-# py2 unicode, py3 str (via hack definition)
+# Check for string types. (py2: unicode, py3: str via type hack
above)
+# or string subclasses. Exclude string subclasses symbol and char.
+if isinstance(k, unicode) and not (type(k) is symbol or type(k) is
char):
+# Convert string subclasses to string
+if not type(k) is unicode:
+self.properties[unicode(k)] = self.properties.pop(k)
continue
Review comment:
But if the above code is actually necessary for some reason, then this
line is no longer necessary. As in context it is just another way of saying
'pass'
##
File path: python/proton/_message.py
##
@@ -90,13 +90,18 @@ def _check(self, err):
def _check_property_keys(self):
for k in self.properties.keys():
-if isinstance(k, unicode):
-# py2 unicode, py3 str (via hack definition)
+# Check for string types. (py2: unicode, py3: str via type hack
above)
+# or string subclasses. Exclude string subclasses symbol and char.
+if isinstance(k, unicode) and not (type(k) is symbol or type(k) is
char):
+# Convert string subclasses to string
+if not type(k) is unicode:
+self.properties[unicode(k)] = self.properties.pop(k)
Review comment:
Why is this even necessary? It's already a (perhaps subclass of)
unicode. won't encode just work directly?
##
File path: python/proton/_message.py
##
@@ -90,13 +90,18 @@ def _check(self, err):
def _check_property_keys(self):
for k in self.properties.keys():
-if isinstance(k, unicode):
-# py2 unicode, py3 str (via hack definition)
+# Check for string types. (py2: unicode, py3: str via type hack
above)
+# or string subclasses. Exclude string subclasses symbol and char.
+if isinstance(k, unicode) and not (type(k) is symbol or type(k) is
char):
+# Convert string subclasses to string
+if not type(k) is unicode:
+self.properties[unicode(k)] = self.properties.pop(k)
continue
-# If key is binary then change to string
-elif isinstance(k, str):
-# py2 str
-self.properties[k.encode('utf-8')] = self.properties.pop(k)
+# If key is binary then change to string. Exclude bytes subclass
decimal128.
+# Mostly for py2 users who encode strings without using the u''
prefix
+# but py3 bytes() type will be converted also
Review comment:
I don't think converting py3 bytes is acceptable as binary is not a good
key (or is it?) it looks like in the previous code it would have triggered an
error - only py2 binary would have been converted.
##
File path: python/proton/_message.py
##
@@ -90,13 +90,18 @@ def _check(self, err):
def _check_property_keys(self):
for k in self.properties.keys():
-if isinstance(k, unicode):
-# py2 unicode, py3 str (via hack definition)
+# Check for string types. (py2: unicode, py3: str via type hack
above)
+# or string subclasses. Exclude string subclasses symbol and char.
+if isinstance(k, unicode) and not (type(k) is symbol or type(k) is
char):
+# Convert string subclasses to string
+if not type(k) is unicode:
+self.properties[unicode(k)] = self.properties.pop(k)
continue
-# If key is binary then change to string
-elif isinstance(k, str):
-# py2 str
-self.properties[k.encode('utf-8')] = self.properties.pop(k)
+# If key is binary then change to string. Exclude bytes subclass
decimal128.
+# Mostly for py2 users who encode strings without using the u''
prefix
+
[jira] [Commented] (PROTON-2237) [python] Non-string message property keys not handled correctly
[ https://issues.apache.org/jira/browse/PROTON-2237?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17176426#comment-17176426 ] ASF GitHub Bot commented on PROTON-2237: kpvdr commented on a change in pull request #256: URL: https://github.com/apache/qpid-proton/pull/256#discussion_r469349354 ## File path: python/proton/_message.py ## @@ -90,13 +90,16 @@ def _check(self, err): def _check_property_keys(self): for k in self.properties.keys(): -if isinstance(k, unicode): -# py2 unicode, py3 str (via hack definition) +# Check for string type. (py2: unicode, py3: str via type hack above) +# String subclasses symbol and char are excluded +# (But so are other string subclasses that would be encoded as type string!) +if type(k) == unicode: Review comment: After discussion, we think that AMQP subclasses of string should be excluded from the conversion. Thus, symbol and char will cause an exception, but other user-defined subclasses will be converted. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > [python] Non-string message property keys not handled correctly > --- > > Key: PROTON-2237 > URL: https://issues.apache.org/jira/browse/PROTON-2237 > Project: Qpid Proton > Issue Type: Bug > Components: python-binding >Reporter: Kim van der Riet >Priority: Major > > The AMQP 1.0 spec allows only string keys for message properties. > Proton's Python binding has a method (_message.py:91 _check_property_keys()) > for checking message property keys, but it does not handle all cases > correctly: > # Proton types Symbol and Char are derived from string, and are allowed in > the test. This results in an illegal encoding. > # Because in Python 2, many coders carelessly use string literals without > the required u'' prefix (and thus results in a bytes type), bytes types are > converted to unicode string types. However, the encode() function is being > used, which simply returns a binary type in Python 2 and raises an error in > Python 3. This should probably be the decode() method, which returns a string > and works for both Python 2 and 3. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[jira] [Commented] (PROTON-2237) [python] Non-string message property keys not handled correctly
[ https://issues.apache.org/jira/browse/PROTON-2237?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17176422#comment-17176422 ] ASF GitHub Bot commented on PROTON-2237: kpvdr commented on a change in pull request #256: URL: https://github.com/apache/qpid-proton/pull/256#discussion_r469348071 ## File path: python/proton/_message.py ## @@ -90,10 +90,12 @@ def _check(self, err): def _check_property_keys(self): for k in self.properties.keys(): -# Check for string type. (py2: unicode, py3: str via type hack above) -# String subclasses symbol and char are excluded -# (But so are other string subclasses that would be encoded as type string!) -if type(k) == unicode: +# Check for string types. (py2: unicode, py3: str via type hack above) +# or string subclasses +if isinstance(k, unicode): +# Convert string subclasses (including proton char and symbol types) to string Review comment: Now that we are specifically excluding symbol and char types, your code applies. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > [python] Non-string message property keys not handled correctly > --- > > Key: PROTON-2237 > URL: https://issues.apache.org/jira/browse/PROTON-2237 > Project: Qpid Proton > Issue Type: Bug > Components: python-binding >Reporter: Kim van der Riet >Priority: Major > > The AMQP 1.0 spec allows only string keys for message properties. > Proton's Python binding has a method (_message.py:91 _check_property_keys()) > for checking message property keys, but it does not handle all cases > correctly: > # Proton types Symbol and Char are derived from string, and are allowed in > the test. This results in an illegal encoding. > # Because in Python 2, many coders carelessly use string literals without > the required u'' prefix (and thus results in a bytes type), bytes types are > converted to unicode string types. However, the encode() function is being > used, which simply returns a binary type in Python 2 and raises an error in > Python 3. This should probably be the decode() method, which returns a string > and works for both Python 2 and 3. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[jira] [Commented] (PROTON-2237) [python] Non-string message property keys not handled correctly
[ https://issues.apache.org/jira/browse/PROTON-2237?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17176421#comment-17176421 ] ASF GitHub Bot commented on PROTON-2237: kpvdr commented on pull request #256: URL: https://github.com/apache/qpid-proton/pull/256#issuecomment-672945072 @jiridanek: Yes, I just added them. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > [python] Non-string message property keys not handled correctly > --- > > Key: PROTON-2237 > URL: https://issues.apache.org/jira/browse/PROTON-2237 > Project: Qpid Proton > Issue Type: Bug > Components: python-binding >Reporter: Kim van der Riet >Priority: Major > > The AMQP 1.0 spec allows only string keys for message properties. > Proton's Python binding has a method (_message.py:91 _check_property_keys()) > for checking message property keys, but it does not handle all cases > correctly: > # Proton types Symbol and Char are derived from string, and are allowed in > the test. This results in an illegal encoding. > # Because in Python 2, many coders carelessly use string literals without > the required u'' prefix (and thus results in a bytes type), bytes types are > converted to unicode string types. However, the encode() function is being > used, which simply returns a binary type in Python 2 and raises an error in > Python 3. This should probably be the decode() method, which returns a string > and works for both Python 2 and 3. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[jira] [Commented] (PROTON-2237) [python] Non-string message property keys not handled correctly
[ https://issues.apache.org/jira/browse/PROTON-2237?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17176396#comment-17176396 ] ASF subversion and git services commented on PROTON-2237: - Commit 08db56e14045896057c483592d0230890643cace in qpid-proton's branch refs/heads/python-check-property-keys from Kim van der Riet [ https://gitbox.apache.org/repos/asf?p=qpid-proton.git;h=08db56e ] PROTON-2237: Finalized handling property keys, added tests for these cases. > [python] Non-string message property keys not handled correctly > --- > > Key: PROTON-2237 > URL: https://issues.apache.org/jira/browse/PROTON-2237 > Project: Qpid Proton > Issue Type: Bug > Components: python-binding >Reporter: Kim van der Riet >Priority: Major > > The AMQP 1.0 spec allows only string keys for message properties. > Proton's Python binding has a method (_message.py:91 _check_property_keys()) > for checking message property keys, but it does not handle all cases > correctly: > # Proton types Symbol and Char are derived from string, and are allowed in > the test. This results in an illegal encoding. > # Because in Python 2, many coders carelessly use string literals without > the required u'' prefix (and thus results in a bytes type), bytes types are > converted to unicode string types. However, the encode() function is being > used, which simply returns a binary type in Python 2 and raises an error in > Python 3. This should probably be the decode() method, which returns a string > and works for both Python 2 and 3. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[jira] [Commented] (PROTON-2237) [python] Non-string message property keys not handled correctly
[ https://issues.apache.org/jira/browse/PROTON-2237?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17176395#comment-17176395 ] ASF subversion and git services commented on PROTON-2237: - Commit 4e19c7ce9c18304d793719d493ce602a6755e271 in qpid-proton's branch refs/heads/python-check-property-keys from Kim van der Riet [ https://gitbox.apache.org/repos/asf?p=qpid-proton.git;h=4e19c7c ] PROTON-2237: Added unit tests to check illegal key types are detected and handled, also subclasses of string type keys are converted to type string > [python] Non-string message property keys not handled correctly > --- > > Key: PROTON-2237 > URL: https://issues.apache.org/jira/browse/PROTON-2237 > Project: Qpid Proton > Issue Type: Bug > Components: python-binding >Reporter: Kim van der Riet >Priority: Major > > The AMQP 1.0 spec allows only string keys for message properties. > Proton's Python binding has a method (_message.py:91 _check_property_keys()) > for checking message property keys, but it does not handle all cases > correctly: > # Proton types Symbol and Char are derived from string, and are allowed in > the test. This results in an illegal encoding. > # Because in Python 2, many coders carelessly use string literals without > the required u'' prefix (and thus results in a bytes type), bytes types are > converted to unicode string types. However, the encode() function is being > used, which simply returns a binary type in Python 2 and raises an error in > Python 3. This should probably be the decode() method, which returns a string > and works for both Python 2 and 3. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[jira] [Commented] (PROTON-2237) [python] Non-string message property keys not handled correctly
[ https://issues.apache.org/jira/browse/PROTON-2237?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17175776#comment-17175776 ] ASF subversion and git services commented on PROTON-2237: - Commit 6eac2102a0e2cec9cae7fb9d5d59a0f6bae345f2 in qpid-proton's branch refs/heads/python-check-property-keys from Kim van der Riet [ https://gitbox.apache.org/repos/asf?p=qpid-proton.git;h=6eac210 ] PROTON-2237: Changed logic of key check so that subclasses of string *except* proton.symbol and proton.char will be encoded as strings > [python] Non-string message property keys not handled correctly > --- > > Key: PROTON-2237 > URL: https://issues.apache.org/jira/browse/PROTON-2237 > Project: Qpid Proton > Issue Type: Bug > Components: python-binding >Reporter: Kim van der Riet >Priority: Major > > The AMQP 1.0 spec allows only string keys for message properties. > Proton's Python binding has a method (_message.py:91 _check_property_keys()) > for checking message property keys, but it does not handle all cases > correctly: > # Proton types Symbol and Char are derived from string, and are allowed in > the test. This results in an illegal encoding. > # Because in Python 2, many coders carelessly use string literals without > the required u'' prefix (and thus results in a bytes type), bytes types are > converted to unicode string types. However, the encode() function is being > used, which simply returns a binary type in Python 2 and raises an error in > Python 3. This should probably be the decode() method, which returns a string > and works for both Python 2 and 3. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[jira] [Commented] (PROTON-2237) [python] Non-string message property keys not handled correctly
[ https://issues.apache.org/jira/browse/PROTON-2237?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17166959#comment-17166959 ] ASF GitHub Bot commented on PROTON-2237: jiridanek commented on pull request #256: URL: https://github.com/apache/qpid-proton/pull/256#issuecomment-665112300 @kpvdr Do you have a test for this change? I think it should be part of the PR. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > [python] Non-string message property keys not handled correctly > --- > > Key: PROTON-2237 > URL: https://issues.apache.org/jira/browse/PROTON-2237 > Project: Qpid Proton > Issue Type: Bug > Components: python-binding >Reporter: Kim van der Riet >Priority: Major > > The AMQP 1.0 spec allows only string keys for message properties. > Proton's Python binding has a method (_message.py:91 _check_property_keys()) > for checking message property keys, but it does not handle all cases > correctly: > # Proton types Symbol and Char are derived from string, and are allowed in > the test. This results in an illegal encoding. > # Because in Python 2, many coders carelessly use string literals without > the required u'' prefix (and thus results in a bytes type), bytes types are > converted to unicode string types. However, the encode() function is being > used, which simply returns a binary type in Python 2 and raises an error in > Python 3. This should probably be the decode() method, which returns a string > and works for both Python 2 and 3. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[jira] [Commented] (PROTON-2237) [python] Non-string message property keys not handled correctly
[ https://issues.apache.org/jira/browse/PROTON-2237?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17149546#comment-17149546 ] ASF GitHub Bot commented on PROTON-2237: kpvdr commented on a change in pull request #256: URL: https://github.com/apache/qpid-proton/pull/256#discussion_r448484945 ## File path: python/proton/_message.py ## @@ -90,13 +90,16 @@ def _check(self, err): def _check_property_keys(self): for k in self.properties.keys(): -if isinstance(k, unicode): -# py2 unicode, py3 str (via hack definition) +# Check for string type. (py2: unicode, py3: str via type hack above) +# String subclasses symbol and char are excluded +# (But so are other string subclasses that would be encoded as type string!) +if type(k) == unicode: Review comment: I did it this way on the understanding that we convert ALL unicode subclasses, including symbol and char, and that this would add to an easier user experience. Certainly it would be more "technically correct" to exclude symbol and char though. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > [python] Non-string message property keys not handled correctly > --- > > Key: PROTON-2237 > URL: https://issues.apache.org/jira/browse/PROTON-2237 > Project: Qpid Proton > Issue Type: Bug > Components: python-binding >Reporter: Kim van der Riet >Priority: Major > > The AMQP 1.0 spec allows only string keys for message properties. > Proton's Python binding has a method (_message.py:91 _check_property_keys()) > for checking message property keys, but it does not handle all cases > correctly: > # Proton types Symbol and Char are derived from string, and are allowed in > the test. This results in an illegal encoding. > # Because in Python 2, many coders carelessly use string literals without > the required u'' prefix (and thus results in a bytes type), bytes types are > converted to unicode string types. However, the encode() function is being > used, which simply returns a binary type in Python 2 and raises an error in > Python 3. This should probably be the decode() method, which returns a string > and works for both Python 2 and 3. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[jira] [Commented] (PROTON-2237) [python] Non-string message property keys not handled correctly
[ https://issues.apache.org/jira/browse/PROTON-2237?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17149544#comment-17149544 ] ASF GitHub Bot commented on PROTON-2237: kpvdr commented on a change in pull request #256: URL: https://github.com/apache/qpid-proton/pull/256#discussion_r448482190 ## File path: python/proton/_message.py ## @@ -90,10 +90,12 @@ def _check(self, err): def _check_property_keys(self): for k in self.properties.keys(): -# Check for string type. (py2: unicode, py3: str via type hack above) -# String subclasses symbol and char are excluded -# (But so are other string subclasses that would be encoded as type string!) -if type(k) == unicode: +# Check for string types. (py2: unicode, py3: str via type hack above) +# or string subclasses +if isinstance(k, unicode): +# Convert string subclasses (including proton char and symbol types) to string +if not type(k) == unicode: Review comment: Agreed. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > [python] Non-string message property keys not handled correctly > --- > > Key: PROTON-2237 > URL: https://issues.apache.org/jira/browse/PROTON-2237 > Project: Qpid Proton > Issue Type: Bug > Components: python-binding >Reporter: Kim van der Riet >Priority: Major > > The AMQP 1.0 spec allows only string keys for message properties. > Proton's Python binding has a method (_message.py:91 _check_property_keys()) > for checking message property keys, but it does not handle all cases > correctly: > # Proton types Symbol and Char are derived from string, and are allowed in > the test. This results in an illegal encoding. > # Because in Python 2, many coders carelessly use string literals without > the required u'' prefix (and thus results in a bytes type), bytes types are > converted to unicode string types. However, the encode() function is being > used, which simply returns a binary type in Python 2 and raises an error in > Python 3. This should probably be the decode() method, which returns a string > and works for both Python 2 and 3. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[jira] [Commented] (PROTON-2237) [python] Non-string message property keys not handled correctly
[ https://issues.apache.org/jira/browse/PROTON-2237?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17149543#comment-17149543 ] ASF GitHub Bot commented on PROTON-2237: kpvdr commented on a change in pull request #256: URL: https://github.com/apache/qpid-proton/pull/256#discussion_r448481904 ## File path: python/proton/_message.py ## @@ -90,10 +90,12 @@ def _check(self, err): def _check_property_keys(self): for k in self.properties.keys(): -# Check for string type. (py2: unicode, py3: str via type hack above) -# String subclasses symbol and char are excluded -# (But so are other string subclasses that would be encoded as type string!) -if type(k) == unicode: +# Check for string types. (py2: unicode, py3: str via type hack above) +# or string subclasses +if isinstance(k, unicode): +# Convert string subclasses (including proton char and symbol types) to string Review comment: It is not logically identical. We want the continue to be reached for all unicode. However, only for subclasses of unicode, we make the conversion. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > [python] Non-string message property keys not handled correctly > --- > > Key: PROTON-2237 > URL: https://issues.apache.org/jira/browse/PROTON-2237 > Project: Qpid Proton > Issue Type: Bug > Components: python-binding >Reporter: Kim van der Riet >Priority: Major > > The AMQP 1.0 spec allows only string keys for message properties. > Proton's Python binding has a method (_message.py:91 _check_property_keys()) > for checking message property keys, but it does not handle all cases > correctly: > # Proton types Symbol and Char are derived from string, and are allowed in > the test. This results in an illegal encoding. > # Because in Python 2, many coders carelessly use string literals without > the required u'' prefix (and thus results in a bytes type), bytes types are > converted to unicode string types. However, the encode() function is being > used, which simply returns a binary type in Python 2 and raises an error in > Python 3. This should probably be the decode() method, which returns a string > and works for both Python 2 and 3. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[jira] [Commented] (PROTON-2237) [python] Non-string message property keys not handled correctly
[ https://issues.apache.org/jira/browse/PROTON-2237?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17138708#comment-17138708 ] ASF GitHub Bot commented on PROTON-2237: astitcher commented on a change in pull request #256: URL: https://github.com/apache/qpid-proton/pull/256#discussion_r436066742 ## File path: python/proton/_message.py ## @@ -90,10 +90,12 @@ def _check(self, err): def _check_property_keys(self): for k in self.properties.keys(): -# Check for string type. (py2: unicode, py3: str via type hack above) -# String subclasses symbol and char are excluded -# (But so are other string subclasses that would be encoded as type string!) -if type(k) == unicode: +# Check for string types. (py2: unicode, py3: str via type hack above) +# or string subclasses +if isinstance(k, unicode): +# Convert string subclasses (including proton char and symbol types) to string Review comment: I really don't understand why this is better than ```if isinstance(k, unicode) and not type(k) is symbol:``` ## File path: python/proton/_message.py ## @@ -90,13 +90,16 @@ def _check(self, err): def _check_property_keys(self): for k in self.properties.keys(): -if isinstance(k, unicode): -# py2 unicode, py3 str (via hack definition) +# Check for string type. (py2: unicode, py3: str via type hack above) +# String subclasses symbol and char are excluded +# (But so are other string subclasses that would be encoded as type string!) +if type(k) == unicode: Review comment: I think do the opposite - check for the specific single type symbol and error out for it - every other string type is allowed. @ssorj what do you think? ## File path: python/proton/_message.py ## @@ -90,10 +90,12 @@ def _check(self, err): def _check_property_keys(self): for k in self.properties.keys(): -# Check for string type. (py2: unicode, py3: str via type hack above) -# String subclasses symbol and char are excluded -# (But so are other string subclasses that would be encoded as type string!) -if type(k) == unicode: +# Check for string types. (py2: unicode, py3: str via type hack above) +# or string subclasses +if isinstance(k, unicode): +# Convert string subclasses (including proton char and symbol types) to string +if not type(k) == unicode: Review comment: Note that ```==``` is not the correct test here ```is``` would be the correct test. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > [python] Non-string message property keys not handled correctly > --- > > Key: PROTON-2237 > URL: https://issues.apache.org/jira/browse/PROTON-2237 > Project: Qpid Proton > Issue Type: Bug > Components: python-binding >Reporter: Kim van der Riet >Priority: Major > > The AMQP 1.0 spec allows only string keys for message properties. > Proton's Python binding has a method (_message.py:91 _check_property_keys()) > for checking message property keys, but it does not handle all cases > correctly: > # Proton types Symbol and Char are derived from string, and are allowed in > the test. This results in an illegal encoding. > # Because in Python 2, many coders carelessly use string literals without > the required u'' prefix (and thus results in a bytes type), bytes types are > converted to unicode string types. However, the encode() function is being > used, which simply returns a binary type in Python 2 and raises an error in > Python 3. This should probably be the decode() method, which returns a string > and works for both Python 2 and 3. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[jira] [Commented] (PROTON-2237) [python] Non-string message property keys not handled correctly
[ https://issues.apache.org/jira/browse/PROTON-2237?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17126098#comment-17126098 ] ASF subversion and git services commented on PROTON-2237: - Commit 65869963ea8b35927e6a0890052c08ca587da022 in qpid-proton's branch refs/heads/python-check-property-keys from Kim van der Riet [ https://gitbox.apache.org/repos/asf?p=qpid-proton.git;h=6586996 ] PROTON-2237: Alternative approach which converts all child classes of string/unicode to the base class, including proton symbol and char types. > [python] Non-string message property keys not handled correctly > --- > > Key: PROTON-2237 > URL: https://issues.apache.org/jira/browse/PROTON-2237 > Project: Qpid Proton > Issue Type: Bug > Components: python-binding >Reporter: Kim van der Riet >Priority: Major > > The AMQP 1.0 spec allows only string keys for message properties. > Proton's Python binding has a method (_message.py:91 _check_property_keys()) > for checking message property keys, but it does not handle all cases > correctly: > # Proton types Symbol and Char are derived from string, and are allowed in > the test. This results in an illegal encoding. > # Because in Python 2, many coders carelessly use string literals without > the required u'' prefix (and thus results in a bytes type), bytes types are > converted to unicode string types. However, the encode() function is being > used, which simply returns a binary type in Python 2 and raises an error in > Python 3. This should probably be the decode() method, which returns a string > and works for both Python 2 and 3. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[jira] [Commented] (PROTON-2237) [python] Non-string message property keys not handled correctly
[ https://issues.apache.org/jira/browse/PROTON-2237?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17124213#comment-17124213 ] ASF GitHub Bot commented on PROTON-2237: codecov-commenter edited a comment on pull request #256: URL: https://github.com/apache/qpid-proton/pull/256#issuecomment-637742528 # [Codecov](https://codecov.io/gh/apache/qpid-proton/pull/256?src=pr&el=h1) Report > Merging [#256](https://codecov.io/gh/apache/qpid-proton/pull/256?src=pr&el=desc) into [master](https://codecov.io/gh/apache/qpid-proton/commit/65a5d9e837cf8de3024e7039dc7f5f2175d13461&el=desc) will **increase** coverage by `0.09%`. > The diff coverage is `100.00%`. [](https://codecov.io/gh/apache/qpid-proton/pull/256?src=pr&el=tree) ```diff @@Coverage Diff @@ ## master #256 +/- ## == + Coverage 83.05% 83.14% +0.09% == Files 338 339 +1 Lines 4143141521 +90 == + Hits3441034523 +113 + Misses 7021 6998 -23 ``` | [Impacted Files](https://codecov.io/gh/apache/qpid-proton/pull/256?src=pr&el=tree) | Coverage Δ | | |---|---|---| | [python/proton/\_message.py](https://codecov.io/gh/apache/qpid-proton/pull/256/diff?src=pr&el=tree#diff-cHl0aG9uL3Byb3Rvbi9fbWVzc2FnZS5weQ==) | `86.69% <100.00%> (ø)` | | | [ruby/tests/test\_interop.rb](https://codecov.io/gh/apache/qpid-proton/pull/256/diff?src=pr&el=tree#diff-cnVieS90ZXN0cy90ZXN0X2ludGVyb3AucmI=) | `95.55% <0.00%> (ø)` | | | [c/src/proactor/epoll.c](https://codecov.io/gh/apache/qpid-proton/pull/256/diff?src=pr&el=tree#diff-Yy9zcmMvcHJvYWN0b3IvZXBvbGwuYw==) | `82.25% <0.00%> (+0.99%)` | :arrow_up: | | [ruby/lib/codec/data.rb](https://codecov.io/gh/apache/qpid-proton/pull/256/diff?src=pr&el=tree#diff-cnVieS9saWIvY29kZWMvZGF0YS5yYg==) | `96.10% <0.00%> (+3.89%)` | :arrow_up: | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/qpid-proton/pull/256?src=pr&el=continue). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://codecov.io/gh/apache/qpid-proton/pull/256?src=pr&el=footer). Last update [65a5d9e...ccbfcf6](https://codecov.io/gh/apache/qpid-proton/pull/256?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments). This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > [python] Non-string message property keys not handled correctly > --- > > Key: PROTON-2237 > URL: https://issues.apache.org/jira/browse/PROTON-2237 > Project: Qpid Proton > Issue Type: Bug > Components: python-binding >Reporter: Kim van der Riet >Priority: Major > > The AMQP 1.0 spec allows only string keys for message properties. > Proton's Python binding has a method (_message.py:91 _check_property_keys()) > for checking message property keys, but it does not handle all cases > correctly: > # Proton types Symbol and Char are derived from string, and are allowed in > the test. This results in an illegal encoding. > # Because in Python 2, many coders carelessly use string literals without > the required u'' prefix (and thus results in a bytes type), bytes types are > converted to unicode string types. However, the encode() function is being > used, which simply returns a binary type in Python 2 and raises an error in > Python 3. This should probably be the decode() method, which returns a string > and works for both Python 2 and 3. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[jira] [Commented] (PROTON-2237) [python] Non-string message property keys not handled correctly
[ https://issues.apache.org/jira/browse/PROTON-2237?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17124210#comment-17124210 ] ASF GitHub Bot commented on PROTON-2237: codecov-commenter commented on pull request #256: URL: https://github.com/apache/qpid-proton/pull/256#issuecomment-637742528 # [Codecov](https://codecov.io/gh/apache/qpid-proton/pull/256?src=pr&el=h1) Report > Merging [#256](https://codecov.io/gh/apache/qpid-proton/pull/256?src=pr&el=desc) into [master](https://codecov.io/gh/apache/qpid-proton/commit/65a5d9e837cf8de3024e7039dc7f5f2175d13461&el=desc) will **increase** coverage by `0.09%`. > The diff coverage is `100.00%`. [](https://codecov.io/gh/apache/qpid-proton/pull/256?src=pr&el=tree) ```diff @@Coverage Diff @@ ## master #256 +/- ## == + Coverage 83.05% 83.14% +0.09% == Files 338 339 +1 Lines 4143141521 +90 == + Hits3441034523 +113 + Misses 7021 6998 -23 ``` | [Impacted Files](https://codecov.io/gh/apache/qpid-proton/pull/256?src=pr&el=tree) | Coverage Δ | | |---|---|---| | [python/proton/\_message.py](https://codecov.io/gh/apache/qpid-proton/pull/256/diff?src=pr&el=tree#diff-cHl0aG9uL3Byb3Rvbi9fbWVzc2FnZS5weQ==) | `86.69% <100.00%> (ø)` | | | [ruby/tests/test\_interop.rb](https://codecov.io/gh/apache/qpid-proton/pull/256/diff?src=pr&el=tree#diff-cnVieS90ZXN0cy90ZXN0X2ludGVyb3AucmI=) | `95.55% <0.00%> (ø)` | | | [c/src/proactor/epoll.c](https://codecov.io/gh/apache/qpid-proton/pull/256/diff?src=pr&el=tree#diff-Yy9zcmMvcHJvYWN0b3IvZXBvbGwuYw==) | `82.25% <0.00%> (+0.99%)` | :arrow_up: | | [ruby/lib/codec/data.rb](https://codecov.io/gh/apache/qpid-proton/pull/256/diff?src=pr&el=tree#diff-cnVieS9saWIvY29kZWMvZGF0YS5yYg==) | `96.10% <0.00%> (+3.89%)` | :arrow_up: | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/qpid-proton/pull/256?src=pr&el=continue). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://codecov.io/gh/apache/qpid-proton/pull/256?src=pr&el=footer). Last update [65a5d9e...ccbfcf6](https://codecov.io/gh/apache/qpid-proton/pull/256?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments). This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > [python] Non-string message property keys not handled correctly > --- > > Key: PROTON-2237 > URL: https://issues.apache.org/jira/browse/PROTON-2237 > Project: Qpid Proton > Issue Type: Bug > Components: python-binding >Reporter: Kim van der Riet >Priority: Major > > The AMQP 1.0 spec allows only string keys for message properties. > Proton's Python binding has a method (_message.py:91 _check_property_keys()) > for checking message property keys, but it does not handle all cases > correctly: > # Proton types Symbol and Char are derived from string, and are allowed in > the test. This results in an illegal encoding. > # Because in Python 2, many coders carelessly use string literals without > the required u'' prefix (and thus results in a bytes type), bytes types are > converted to unicode string types. However, the encode() function is being > used, which simply returns a binary type in Python 2 and raises an error in > Python 3. This should probably be the decode() method, which returns a string > and works for both Python 2 and 3. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[jira] [Commented] (PROTON-2237) [python] Non-string message property keys not handled correctly
[ https://issues.apache.org/jira/browse/PROTON-2237?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17124097#comment-17124097 ] ASF GitHub Bot commented on PROTON-2237: kpvdr opened a new pull request #256: URL: https://github.com/apache/qpid-proton/pull/256 Proposed fix for PROTON-2237. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > [python] Non-string message property keys not handled correctly > --- > > Key: PROTON-2237 > URL: https://issues.apache.org/jira/browse/PROTON-2237 > Project: Qpid Proton > Issue Type: Bug > Components: python-binding >Reporter: Kim van der Riet >Priority: Major > > The AMQP 1.0 spec allows only string keys for message properties. > Proton's Python binding has a method (_message.py:91 _check_property_keys()) > for checking message property keys, but it does not handle all cases > correctly: > # Proton types Symbol and Char are derived from string, and are allowed in > the test. This results in an illegal encoding. > # Because in Python 2, many coders carelessly use string literals without > the required u'' prefix (and thus results in a bytes type), bytes types are > converted to unicode string types. However, the encode() function is being > used, which simply returns a binary type in Python 2 and raises an error in > Python 3. This should probably be the decode() method, which works for both > Python 2 and 3. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[jira] [Commented] (PROTON-2237) [python] Non-string message property keys not handled correctly
[ https://issues.apache.org/jira/browse/PROTON-2237?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17124090#comment-17124090 ] ASF subversion and git services commented on PROTON-2237: - Commit ccbfcf659d19c0b7917beed823432ba102743386 in qpid-proton's branch refs/heads/python-check-property-keys from Kim van der Riet [ https://gitbox.apache.org/repos/asf?p=qpid-proton.git;h=ccbfcf6 ] PROTON-2237: Correct checking of Proton message property keys > [python] Non-string message property keys not handled correctly > --- > > Key: PROTON-2237 > URL: https://issues.apache.org/jira/browse/PROTON-2237 > Project: Qpid Proton > Issue Type: Bug > Components: python-binding >Reporter: Kim van der Riet >Priority: Major > > The AMQP 1.0 spec allows only string keys for message properties. > Proton's Python binding has a method (_message.py:91 _check_property_keys()) > for checking message property keys, but it does not handle all cases > correctly: > # Proton types Symbol and Char are derived from string, and are allowed in > the test. This results in an illegal encoding. > # Because in Python 2, many coders carelessly use string literals without > the required u'' prefix (and thus results in a bytes type), bytes types are > converted to unicode string types. However, the encode() function is being > used, which simply returns a binary type in Python 2 and raises an error in > Python 3. This should probably be the decode() method, which works for both > Python 2 and 3. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
