Skip to content
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

Fails with python 3.6.3 on windows #17

Closed
rkhwaja opened this issue Oct 10, 2017 · 11 comments · Fixed by #18
Closed

Fails with python 3.6.3 on windows #17

rkhwaja opened this issue Oct 10, 2017 · 11 comments · Fixed by #18

Comments

@rkhwaja
Copy link

rkhwaja commented Oct 10, 2017

I get the following when calling send2trash on python 3.6.3 on windows 7:
File "...\AppData\Local\Programs\Python\Python36\lib\site-packages\send2trash\plat_win.py", line 49, in send2trash fileop.pFrom = LPCWSTR(path + '\0') ValueError: embedded null character

I'm pretty sure it didn't fail until I switched to 3.6.3. My guess is that this fix is responsible.

@ghost
Copy link

ghost commented Oct 10, 2017

Thanks for the report. I don't use Windows, however, so I'll have to rely on a PR from someone else.

Something tells me that simply removing that + '\0' will break stuff, but I can't remember why it's there.

@rkhwaja
Copy link
Author

rkhwaja commented Oct 10, 2017

Yes - I tried that and the win32 call itself returns an error (87 - invalid parameter) - probably because it's expecting a null-terminated string???

@ghost
Copy link

ghost commented Oct 10, 2017

Oh, I remember: pFrom has to be double nul terminated. I don't know how we'll go around that...

@asweigart
Copy link

Is this a problem with certain file names or files with certain content? I'm trying to recreate this issue on Windows 7 with Python 3.6.2 using send2trash 1.4.1 but can't. Can you post the file you are trying to delete? I'm on Windows and could take a look at this issue.

@rkhwaja
Copy link
Author

rkhwaja commented Oct 26, 2017

It's only on Python 3.6.3. The file content doesn't matter.

@arsenetar
Copy link
Owner

arsenetar commented Nov 1, 2017

It is definitely a 3.6.3 (over 3.6.2) issue, there is a possible work-around I have found, but I need to verify that it works as expected reliably. If you want to reproduce the issue on windows all you need to do is:

from send2trash import send2trash
send2trash('pathtosomefile')

Workaround uses ctypes functions to create a buffer and write the null characters into the buffer then use a LPWSTR() cast on the address of the buffer. It seems to work in initial tests. Putting the code inline yeilds:

from ctypes import create_unicode_buffer, addressof, sizeof
from ctypes.wintypes import LPCWSTR
# create buffer of c_wchar[] LPCWSTR is based on this type
# uses length of string + space for two null termination characters
buffer = create_unicode_buffer(path, len(path)+2)
# explicitly set the last two characters
# (not sure if it is possible for them to be non null at this point
buffer[len(path)] = '\0'
buffer[len(path)+1] = '\0'
fileop.pFrom = LPWSTR(addressof(buffer))

I will try getting around to some more verification work but this seems to be working.

Edit: typo in code

@ghost
Copy link

ghost commented Nov 1, 2017

@arsenetar this looks like a good workaround. If you say it works, then I say we go for it. Could you please add a comment with a link to this issue in it? Future readers might not understand the purpose of this.

@arsenetar
Copy link
Owner

@hsoft Yeah I can added that comment information. I would like to look into create_unicode_buffer() to see if the two buffer[index] = '\0' lines are redundant as I have only seen null characters in them at this point.

@ghost
Copy link

ghost commented Nov 1, 2017

@rkhwaja could you please confirm the fix?

@rkhwaja
Copy link
Author

rkhwaja commented Nov 1, 2017

Works great - thanks.

@MagTun
Copy link

MagTun commented May 20, 2019

Just for the latecomers like me, upgrading to version 1.5.0 fixed it:
pip install 'send2trash==1.5.0'

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants