Skip to content

Wrong std::string overload called when given numpy.ndarray argument #685

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
pschella opened this issue Feb 21, 2017 · 2 comments
Closed

Wrong std::string overload called when given numpy.ndarray argument #685

pschella opened this issue Feb 21, 2017 · 2 comments

Comments

@pschella
Copy link
Contributor

When an overload is present that takes a std::string argument, this overload is also (wrongly) called when given a numpy.ndarray.

See attached unit test patch to latest master.
broken_overload.txt
@dean0x7d
Copy link
Member

Looks like it only affects Python 2. PyUnicode_FromObject might be a bit too permissive and accepts ndarray via the buffer protocol:

https://docs.python.org/2.7/c-api/unicode.html#c.PyUnicode_FromObject
String and other char buffer compatible objects are decoded according to the given encoding

@jagerman Perhaps an additional PyString_Check is needed for Python 2?

jagerman added a commit to jagerman/pybind11 that referenced this issue Feb 23, 2017
@jagerman
Make string conversion stricter
927a19f 
The string conversion logic added in PR pybind#624 for all std::basic_strings
was using the old std::wstring logic, but that was underused and turns
out to have hade a bug in accepting almost anything convertible to
the previous std::string logic by only accepting unicode or byte/string
(Python 3/2) types.

Fixes pybind#685.
jagerman added a commit to jagerman/pybind11 that referenced this issue Feb 23, 2017
@jagerman
Make string conversion stricter
df15e13 
The string conversion logic added in PR pybind#624 for all std::basic_strings
was derived from the old std::wstring logic, but that was underused and
turns out to have had a bug in accepting almost anything convertible to
unicode, while the previous std::string logic was much stricter.  This
restores the previous std::string logic by only allowing actual unicode
or string types.

Fixes pybind#685.
@jagerman jagerman mentioned this issue Feb 23, 2017
Merged
wjakob pushed a commit that referenced this issue Feb 24, 2017
@jagerman @wjakob
Make string conversion stricter (#695)
ee2e5a5 
* Make string conversion stricter

The string conversion logic added in PR #624 for all std::basic_strings
was derived from the old std::wstring logic, but that was underused and
turns out to have had a bug in accepting almost anything convertible to
unicode, while the previous std::string logic was much stricter.  This
restores the previous std::string logic by only allowing actual unicode
or string types.

Fixes #685.

* Added missing 'requires numpy' decorator

(I forgot that the change to a global decorator here is in the
not-yet-merged Eigen PR)
@pschella
Copy link
Contributor Author

pschella commented Feb 27, 2017 •
edited
Loading

Any chance we can get a new point release with the fix for this, #597, #688 and potentially other recent fixes?

@pschella pschella mentioned this issue Feb 27, 2017
Merged
@rwgk rwgk mentioned this issue Feb 9, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@pschella @dean0x7d