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.
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
zipimport.zipimporter.get_data() crashes when path.replace() returns a non-str #75472
Comments
on Windows, assuming the file 'foo.zip' exists, the following would cause an import zipimport
class BadStr(str):
def replace(self, old, new):
return 42 zipimport.zipimporter('foo.zip').get_data(BadStr('bar')) this is because zipimport_zipimporter_get_data_impl() assumes that |
I don't think this is a bad thing. This is duck-typing at work to be flexible where the type doesn't really matter, just the provided interface. If someone happens to provide an object that doesn't do the right thing then that's their fault for using the wrong object and the resulting exception that will occur from a bad path is acceptable. I appreciate the work to do a PR, Oren, but I'm closing this issue as "rejected". (And if it makes you feel any better, you helped find a bug in the Bedevere bot. :) . |
I understand that our goal is to make Python better, not to make me happier :) anyway, I haven't checked, but I am quite sure that similar code might |
If it will crash the interpreter then that's different. If that turns out to be the case then please come back here and we can rework the issue and PR to prevent that by being better about checking return values or resulting exception cases. |
just checked, and indeed on my Windows 10 the original code I posted here The patch in the PR undermines duck-typing, and that's why I added a comment do you think I would update the PR to do that? |
Could you paste in what the crash looks like? E.g. is it a segfault, SystemError, etc.? And you can tweak the PR or just open a new one if you would rather that fixes the crash itself without taking away duck typing. |
Oren already fixed several similar problems and he is aware of difference between a crash and an exception. The problem is Windows specific. The current code uses Unicode C API with the result of calling the replace method. This can cause a segfault (or may be an assertion failure in debug build). The proposed solution LGTM. It undermines duck-typing, but I think this doesn't break any existing code. The duck-typing was not intentional and is Windows-only. |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: