Skip to content

[lldb] Make sure completions don't end with newlines #117054

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

Merged
merged 1 commit into from
Nov 21, 2024

Conversation

JDevlieghere
Copy link
Member

The logic that prints completions and their descriptions assumes neither the completion itself nor the description ends with a newline. I considered making this an assert, but decided against it as completions can indirectly come from user input (e.g. oddly crafted names). Instead, avoid the potential for mistakes by defensively stripping them.

The logic that prints completions and their descriptions assumes neither
the completion itself nor the description ends with a newline. I
considered making this an assert, but decided against it as completions
can indirectly come from user input (e.g. oddly crafted names). Instead,
avoid the potential for mistakes by defensively stripping them.
@JDevlieghere JDevlieghere requested a review from labath November 20, 2024 21:24
@llvmbot llvmbot added the lldb label Nov 20, 2024
@llvmbot
Copy link
Member

llvmbot commented Nov 20, 2024

@llvm/pr-subscribers-lldb

Author: Jonas Devlieghere (JDevlieghere)

Changes

The logic that prints completions and their descriptions assumes neither the completion itself nor the description ends with a newline. I considered making this an assert, but decided against it as completions can indirectly come from user input (e.g. oddly crafted names). Instead, avoid the potential for mistakes by defensively stripping them.


Full diff: https://github.com/llvm/llvm-project/pull/117054.diff

1 Files Affected:

  • (modified) lldb/include/lldb/Utility/CompletionRequest.h (+2-2)
diff --git a/lldb/include/lldb/Utility/CompletionRequest.h b/lldb/include/lldb/Utility/CompletionRequest.h
index 650158a197dbd9..865d6db5762984 100644
--- a/lldb/include/lldb/Utility/CompletionRequest.h
+++ b/lldb/include/lldb/Utility/CompletionRequest.h
@@ -52,8 +52,8 @@ class CompletionResult {
   public:
     Completion(llvm::StringRef completion, llvm::StringRef description,
                CompletionMode mode)
-        : m_completion(completion.str()), m_descripton(description.str()),
-          m_mode(mode) {}
+        : m_completion(completion.rtrim().str()),
+          m_descripton(description.rtrim().str()), m_mode(mode) {}
     const std::string &GetCompletion() const { return m_completion; }
     const std::string &GetDescription() const { return m_descripton; }
     CompletionMode GetMode() const { return m_mode; }

@labath
Copy link
Collaborator

labath commented Nov 21, 2024

I get the "description" part, but I'm somewhat confused about the "completion". How does a newline (trailing or not) get there? Like, you can't actually type something which includes a newline, right? I'm wondering if we should disallow these at a higher level...

@JDevlieghere
Copy link
Member Author

I get the "description" part, but I'm somewhat confused about the "completion". How does a newline (trailing or not) get there? Like, you can't actually type something which includes a newline, right? I'm wondering if we should disallow these at a higher level...

I didn't actually find a completion where that was the case, but it seemed simple enough to be defensive for both. Like I said in the description, I considered an assertion but technically this is user input. I didn't actually try this, but what I was thinking of is something like a newline in DW_AT_name or something. Obviously that's not something reasonable tools would generate, but input is input...

Copy link
Collaborator

@labath labath left a comment

Choose a reason for hiding this comment

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

Okay, so you're saying that we could hit this if we're completing an expression (variable name) and a particularly evil compiler puts a newline into the name. Makes sense, sort of.

@JDevlieghere JDevlieghere merged commit 4b5a8d6 into llvm:main Nov 21, 2024
9 checks passed
@JDevlieghere JDevlieghere deleted the strip-completions branch November 21, 2024 16:00
JDevlieghere added a commit to swiftlang/llvm-project that referenced this pull request Nov 21, 2024
The logic that prints completions and their descriptions assumes neither
the completion itself nor the description ends with a newline. I
considered making this an assert, but decided against it as completions
can indirectly come from user input (e.g. oddly crafted names). Instead,
avoid the potential for mistakes by defensively stripping them.

(cherry picked from commit 4b5a8d6)
JDevlieghere added a commit to swiftlang/llvm-project that referenced this pull request Dec 16, 2024
…4b5a8d683589

[🍒 swift/release/6.1] [lldb] Make sure completions don't end with newlines (llvm#117054)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants