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.

Sign up for GitHub

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

Jump to bottom

Terminal link validation interferes with codespace connection for very long wrapped lines #139593

Closed
Tyriar opened this issue Dec 21, 2021 · 5 comments · Fixed by #140936 or #141755
Closed

Terminal link validation interferes with codespace connection for very long wrapped lines #139593

Tyriar opened this issue Dec 21, 2021 · 5 comments · Fixed by #140936 or #141755
Assignees
@Tyriar
Labels
bug Issue identified by VS Code Team member as probable bug insiders-released Patch has been released in VS Code Insiders remote Remote system operations issues terminal-links verified Verification succeeded
Milestone
February 2022

Comments

@Tyriar
Copy link
Member

Tyriar commented Dec 21, 2021

Repro:

Echo a really long wrapped line, this whole line will get split up via regex and each word that looks like a path will be validated, requiring a round trip to the remote.

Solution:

We should catch this case and restrict the amount of characters we search back/forward for long wrapped lines.
@Tyriar Tyriar added bug Issue identified by VS Code Team member as probable bug remote Remote system operations issues terminal-links labels Dec 21, 2021
@Tyriar Tyriar added this to the January 2022 milestone Dec 21, 2021
@Tyriar Tyriar mentioned this issue Dec 21, 2021
Closed
meganrogge added a commit that referenced this issue Jan 18, 2022
@meganrogge
work on #139593
d8549ec
@meganrogge meganrogge mentioned this issue Jan 18, 2022
Merged
@rzhao271
Copy link
Contributor

rzhao271 commented Feb 1, 2022

What are some steps to verify this issue? I tried copy-pasting a link many times and echoing that. I noticed that I couldn't ctrl+click the links near the end, but there was still some lag. The lag might have been from echoing all that text, though.

@rzhao271 rzhao271 added the verification-steps-needed Steps to verify are needed for verification label Feb 1, 2022
@meganrogge
Copy link
Contributor

meganrogge commented Feb 1, 2022 •
edited

to verify this, you'll need a link that's longer than the length of the terminal buffer. to find the length of the buffer:

  1. run echo "columns: `tput cols` Rows: `tput lines` " in the terminal
  2. multiply cols*rows then add an arbitrary additional amount to exceed this
  3. use https://charcounter.com/en/ or similar to make a link that many chars long
  4. in a remote workspace, echo that link in the terminal - not sure if this is fixed in codespaces yet
  5. hover and click the link
  6. use char counter to check that the link that's in the quick-pick is no longer than the buffer length

@meganrogge meganrogge removed the verification-steps-needed Steps to verify are needed for verification label Feb 1, 2022
@meganrogge
Copy link
Contributor

@rzhao271 added steps

@rzhao271 rzhao271 added the verified Verification succeeded label Feb 1, 2022
@rzhao271
Copy link
Contributor

rzhao271 commented Feb 1, 2022

Not working for me. I tried codespaces and dev containers.
Example link in dev containers:

node:/workspaces/dc-nodejs$ echo workspaces/dc-nodejs/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.txt
workspaces/dc-nodejs/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.txt
node:/workspaces/dc-nodejs$ echo "columns: `tput cols` Rows: `tput lines` "
columns: 77 Rows: 12

The link in the quickpick was 1172 characters long. The buffer is 924 characters long.

@rzhao271 rzhao271 reopened this Feb 1, 2022
@rzhao271 rzhao271 added verification-found Issue verification failed and removed verified Verification succeeded labels Feb 1, 2022
@meganrogge meganrogge removed the verification-found Issue verification failed label Feb 1, 2022
@meganrogge meganrogge modified the milestones: January 2022, February 2022 Feb 1, 2022
@Tyriar
Copy link
Member Author

Tyriar commented Feb 1, 2022

I'll have a look at this at part of #141755

@Tyriar Tyriar assigned Tyriar and unassigned meganrogge Feb 1, 2022
Tyriar added a commit that referenced this issue Feb 1, 2022
@Tyriar
Filter out links based on count and length
5d74271 
Part of #139593
Tyriar added a commit that referenced this issue Feb 2, 2022
@Tyriar
Put resolved link rate limit protections in place for local links
7ca7fbc 
Fixes #139593
@Tyriar Tyriar mentioned this issue Feb 2, 2022
Merged
@connor4312 connor4312 added the verified Verification succeeded label Feb 24, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Mar 19, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@Tyriar Tyriar

Labels
bug Issue identified by VS Code Team member as probable bug insiders-released Patch has been released in VS Code Insiders remote Remote system operations issues terminal-links verified Verification succeeded
Projects
None yet
Milestone
February 2022
Development

Successfully merging a pull request may close this issue.

cap link size microsoft/vscode
Terminal link refactors microsoft/vscode
5 participants
@Tyriar @connor4312 @rzhao271 @meganrogge and others