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

getLanguageDesc bug #146

Closed
pryrt opened this issue Mar 9, 2020 · 5 comments
Closed

getLanguageDesc bug #146

pryrt opened this issue Mar 9, 2020 · 5 comments
Labels
Milestone

Comments

@pryrt
Copy link
Contributor

pryrt commented Mar 9, 2020

@Ekopalypse and I have noticed that there's a potential bug in PythonScript's implementation of getLanguageDesc: https://community.notepad-plus-plus.org/post/51274

We can get it to reliably crash using the brief script:

from Npp import notepad, LANGTYPE

LANGUAGES = LANGTYPE.values.values()
for language in LANGUAGES:
    print(notepad.getLanguageDesc(language))

When I run the equivalent script using my Perl Win32::Mechanize::NotepadPlusPlus to call the underlying NPPM_GETLANGUAGEDESC (or if Eko uses python's ctypes.windll.user32.SendMessageW to do the same), it works reliably.

@Ekopalypse
Copy link
Contributor

Not sure, but I assume that here one needs to add 1 to the buffer allocation.
but I might be wrong as I don't have sufficient c++ knowledge.

@pryrt
Copy link
Contributor Author

pryrt commented Mar 9, 2020

well, getPluginHomePath does use [size+1], so that's likely it.

(getPluginHomePath, getLanguageName, and getLanguageDesc are the only three that use the idiom of size = callNotepad(...) for allocating the size, so hopefully the two language functions are the only two with that bug.)

@chcg chcg added the Bug label Apr 10, 2020
@chcg
Copy link
Collaborator

chcg commented Apr 10, 2020

See N++ description of how to use NPPM_GETLANGUAGEDESC and NPPM_GETLANGUAGENAME and the size+1 was missing as you already found out.
Thanks.

There are tests for that:
https://github.com/bruderstein/PythonScript/blob/master/PythonScript/python_tests/tests/NotepadWrapperTestCase.py#L795-L824
,but last time I run it I think there was no issue found, so probably also the testcase needs to be checked.

chcg added a commit that referenced this issue Apr 10, 2020
missing +1 for the allocated buffer for NPPM_GETLANGUAGENAME and NPPM_GETLANGUAGEDESC
@chcg chcg added this to the v1.5 milestone Apr 10, 2020
@Ekopalypse
Copy link
Contributor

The test itself seems to be fine.
As far as I understand, boost:.python truncates the string by \0,
so there is no way for the test to see if it has been allocated correctly or not - I suppose.

@chcg
Copy link
Collaborator

chcg commented Apr 21, 2020

@chcg chcg closed this as completed Apr 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants