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

Prevent undefined behavior when using QString::chopped #2087

Closed

Conversation

Liderate
Copy link
Contributor

@Liderate Liderate commented Aug 2, 2024

This small change is based on a note in the Qt documentation for QString::chopped which states "The behavior is undefined if len is negative or greater than size()." I did not see any other instance of chopped in the codebase that needs changed.

@Holt59
Copy link
Member

Holt59 commented Aug 3, 2024

I don't think QString::chopped should be used here, this looks really weird, we probably want QString::mid.

@Holt59
Copy link
Member

Holt59 commented Aug 3, 2024

According to 9dbac13, chopped() was added to "Limit the download file description to 4096 characters", but that is not what chopped does so clearly this should be changed to simply info->description.mid(0, 4096).

@Holt59
Copy link
Member

Holt59 commented Aug 3, 2024

I made a separate PR for this with mid. For information, I looked at Qt sources, and in 6.7.1, chopped(4096) was equivalent to sliced(0, size() - 4096) which is equivalent to QString(begin(), size() - 4096), so

  • when size() would be less than 4096 (most of the time since according to the previous commit, Nexus description are limited to 255 characters), this is equivalent of QString(begin(), -1) so the size is computed from the QString and the full string is returned - So not "UB" as per the C++ standard.
  • when size() would be greater than 4096, this would basically keep the left-most size() - 4096 characters, so pretty weird output (also in that case, the original description is probably already bad).

@Holt59 Holt59 closed this Aug 3, 2024
@Liderate
Copy link
Contributor Author

Liderate commented Aug 4, 2024

That makes sense. I appreciate the detailed response.

@Liderate Liderate deleted the chopped-undefined-behavior branch August 4, 2024 18:31
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