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

Pasting non-text input crashes Minetest #14150

Closed
xenonca opened this issue Dec 22, 2023 · 10 comments · Fixed by minetest/irrlicht#271
Closed

Pasting non-text input crashes Minetest #14150

xenonca opened this issue Dec 22, 2023 · 10 comments · Fixed by minetest/irrlicht#271
Labels
Bug Issues that were confirmed to be a bug Regression Something that used to work no longer does Windows

Comments

@xenonca
Copy link

xenonca commented Dec 22, 2023

Minetest version

Minetest 5.8.0

Active renderer

OpenGL 4.6

Irrlicht device

No response

Operating system and version

Windows 11

CPU model

No response

GPU model

No response

OpenGL version

4.6

Summary

When trying to paste something that is not text (e.g. a file) into a formspec using Crtl + C and Ctrl + V Minetest will crash with no errors in the logs.

Steps to reproduce

Copy any file with Ctrl + C. Try to paste it into a formspec, e.g. a sign input field, with Ctrl + V.

@xenonca xenonca added the Unconfirmed bug Bug report that has not been confirmed to exist/be reproducible label Dec 22, 2023
@Desour
Copy link
Member

Desour commented Dec 22, 2023

Can't reproduce on linux with SDL2 irrlicht device.

@Desour Desour added the Windows label Dec 22, 2023
@rewired-X
Copy link

I achieved a similar result, although I am not certain if I can reproduce it the exact same way I did it.
I tried to copy-paste coords from Discord to in-game, and crashed 3 times.
The fourth time I tried, I copied the coords again and pasted, this time it worked.

@fluxionary
Copy link
Contributor

@rewired-X can you post the details of your OS and build? (minetest --version)

@srifqi
Copy link
Contributor

srifqi commented Jan 3, 2024

I can confirm this issue on Windows 10 (10.0.19045). This issue does not occur in 5.7.0 or older.

@srifqi srifqi added Bug Issues that were confirmed to be a bug Regression Something that used to work no longer does and removed Unconfirmed bug Bug report that has not been confirmed to exist/be reproducible labels Jan 3, 2024
@srifqi
Copy link
Contributor

srifqi commented Jan 3, 2024

This issue is caused by minetest/irrlicht#249 (specifically minetest/irrlicht@c766c3a).

Stack trace:

/* 7 */ irr::core::wStringToUTF8(class irr::core::string<char> &,wchar_t const *)
/* 6 */ irr::COSOperator::getTextFromClipboard(void)
/* 5 */ irr::gui::CGUIEditBox::processKey(struct irr::SEvent const &)
/* 4 */ irr::gui::CGUIEditBox::OnEvent(struct irr::SEvent const &)
/* 3 */ irr::gui::CGUIEnvironment::postEventFromUser(struct irr::SEvent const &)
/* 2 */ irr::CIrrDeviceStub::postEventFromUser(struct irr::SEvent const &)
/* 1 */ WndProc(struct HWND__ *,unsigned int,unsigned __int64,__int64)

Possible fix:

diff --git a/source/Irrlicht/COSOperator.cpp b/source/Irrlicht/COSOperator.cpp
index d2b3b8d..33b07ec 100644
--- a/source/Irrlicht/COSOperator.cpp
+++ b/source/Irrlicht/COSOperator.cpp
@@ -162,6 +162,9 @@ const c8* COSOperator::getTextFromClipboard() const
 	wchar_t * buffer = 0;
 
 	HANDLE hData = GetClipboardData( CF_UNICODETEXT );
+	if (hData == NULL) // Probably not in Unicode text format
+		return 0;
+
 	buffer = (wchar_t*) GlobalLock( hData );
 
 	core::wStringToUTF8(ClipboardBuf, buffer);

Documentation for GetClipboardData()

@grorp
Copy link
Member

grorp commented Jan 3, 2024

So this doesn't happen with USE_SDL2=TRUE / CIrrDeviceSDL, right? (will become the default on Windows with 5.9.0)

(Also, I'm wondering why the missing null check didn't cause problems before minetest/irrlicht@c766c3a.)

@srifqi
Copy link
Contributor

srifqi commented Jan 4, 2024

So this doesn't happen with USE_SDL2=TRUE / CIrrDeviceSDL, right?

Yes, this issue does not occur using SDL2.

(Also, I'm wondering why the missing null check didn't cause problems before minetest/irrlicht@c766c3a.)

My guess is that the old code checks for null-ness of source parameter.

static inline size_t wStringToMultibyte(string<c8>& destination, const wchar_t* source)
{
	const u32 s = source ? (u32)wcslen(source) : 0;
	return wStringToMultibyte(destination, source, s);
}
static size_t wStringToMultibyte(string<c8>& destination, const wchar_t* source, u32 sourceSize)
{
	if ( sourceSize )
	{
		// ...
	}
	else
	{
		destination.clear();
		return 0;
	}
}

@rewired-X
Copy link

@fluxionary live from the windows command prompt :P
Screenshot (1309)

@grorp
Copy link
Member

grorp commented Jan 4, 2024

Possible fix: [...]

Alright, do you want to open a PR?

@srifqi
Copy link
Contributor

srifqi commented Jan 4, 2024

do you want to open a PR?

I am planning to do it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Issues that were confirmed to be a bug Regression Something that used to work no longer does Windows
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants