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

7670 - Fixed Windows chromium-args 260 character limit #7671

Open
wants to merge 1 commit into
base: nw51
Choose a base branch
from

Conversation

sirisian
Copy link

@sirisian sirisian commented Feb 5, 2021

#7670

Untested change for handling ExpandEnvironmentStrings on Windows. The original code limited chromium-args to 260 characters.

Note, I haven't tried to compile this, and my C++ is rusty. Figured I'd get this started to speed along a fix. It might be fine.

Untested change for handling ExpandEnvironmentStrings on Windows. The original code limited chromium-args to 260 characters.

Note, I haven't tried to compile this, and my C++ is rusty. Figured I'd get this started to speed along a fix.
Copy link
Member

@rogerwang rogerwang left a comment

Choose a reason for hiding this comment

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

The do {} while (true) loop is not needed, isn't it?

@sirisian
Copy link
Author

sirisian commented Feb 12, 2021

ExpandEnvironmentStrings returns the size of the expected character array, so if it's say 10K characters that are required for szEnvPath then it needs to be reallocated on the stack to the size of the returned value. I used a loop here to rerun ExpandEnvironmentStrings with the resized character array. This seemed like the most elegant way to write it since the return value needs to be checked again.

@zkrige
Copy link

zkrige commented Sep 25, 2024

Is this PR still valid? this bug is still present in 0.91.0

@sirisian
Copy link
Author

sirisian commented Oct 1, 2024

@rogerwang Feel free to close this and rewrite it without the do while loop. Glancing at the code you're right that I overcomplicated it.

You can actually just call the function with nullptr and 0 to get back the required size, then just allocated to that size. Something like this:

DWORD requiredSize = ExpandEnvironmentStringsW(input.c_str(), nullptr, 0);

if (requiredSize > 0) {
    std::wstring output(requiredSize, L'\0');
    DWORD expandedSize = ExpandEnvironmentStringsW(input.c_str(), &output[0], requiredSize);
    
    if (expandedSize > 0 && expandedSize <= requiredSize) {
        // Trim null terminator
        output.resize(expandedSize - 1);
        // ...
    } else {
        // Handle error
    }
} else {
    // Handle error
}

@zkrige
Copy link

zkrige commented Oct 1, 2024

This bug is still present in 0.92.0 - it would be fantastic if it could be resolved soonest

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.

3 participants