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

fix: refuse to open overly long URLs #70

Merged
merged 1 commit into from
Jan 28, 2025

Conversation

zao
Copy link
Contributor

@zao zao commented Jan 24, 2025

ShellExecute on Wine can crash if you feed it a too long URL and some browsers have been documented to have a fairly low max limit of 2083 characters.

This rejects URLs that are longer than 2048 characters.

@zao
Copy link
Contributor Author

zao commented Jan 24, 2025

This needs testing on affected Linux systems running Wine to see if the problem is mitigated, as the limits on proper Windows are higher.

@zao
Copy link
Contributor Author

zao commented Jan 25, 2025

Experimental results on Wine 10 on Arch puts the upper limit for me at 1575. This could be very system and environment dependent. Couldn't find any obvious cause in Wine.

Copy link
Member

@Wires77 Wires77 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a quick test on Windows (non-Wine) shows that trade queries can easily be up to almost 3000 characters and still open fine. This hardcoded limit isn't going to work then without some significant changes on the Lua side.

Is it easy enough to test this with xdg-open instead?

The de facto limit for URL lengths under Wine on Linux seems to be
somewhere around 1500 bytes, refuse to open the URL and return a reason
as a string through the Lua API call.
@zao zao force-pushed the fix/check-url-length branch from ee4c92f to 00e0bef Compare January 28, 2025 10:33
@zao
Copy link
Contributor Author

zao commented Jan 28, 2025

Starting native programs when running under Wine seems to be fairly unsupported.
Opted instead to add a rejection limit when detecting Wine on Linux/MacOS, adding an error return value for the Lua OpenURL binding.

@Wires77 Wires77 merged commit b95f1cc into PathOfBuildingCommunity:master Jan 28, 2025
1 check passed
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 this pull request may close these issues.

2 participants