Skip to content

Conversation

@somanath-goudar
Copy link

@somanath-goudar somanath-goudar commented Nov 6, 2023

This PR fixes issue with long query for search box

Pull Request Type

  • Bugfix

Related issue

closes #940

Description

Findings:

  1. There is a limit for the number of character a query can have.
  2. If the query exceeds the limit either 413 error or a bad request is thrown
  3. From my testing, I have found that the character limit is 100 characters
  4. To fix this issue, I have limited the number of characters a user can enter inside the search box

Screenshots

Screen.Recording.2023-11-06.at.6.55.13.PM.mov

@github-actions github-actions bot added the PR: waiting for review For PRs that are complete, tested, and ready for review label Nov 6, 2023
@FreeTubeBot FreeTubeBot enabled auto-merge (squash) November 6, 2023 13:29
Copy link
Member

@PikachuEXE PikachuEXE left a comment

Choose a reason for hiding this comment

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

Please address the warnings from lint first

auto-merge was automatically disabled November 7, 2023 03:11

Head branch was pushed to by a user without write access

@FreeTubeBot FreeTubeBot enabled auto-merge (squash) November 7, 2023 03:11
@somanath-goudar
Copy link
Author

@PikachuEXE Lint warnings are fixed. Please review

@PrestonN
Copy link
Member

PrestonN commented Nov 7, 2023

Hey there! While your PR does fix the issue with searching, it unfortunately creates a different problem.

Our search bar handles a number of different functions outside of searching. The main one is that you can paste a YouTube URL into the search bar and FreeTube will parse that URL to take you directly to the video. Sometimes, this URL will be an Invidious URL. Other times, the URL will include playlist information and we'll show that as well once we've taken you to your video. It could also be a combination of the 2.

The TLDR is that limiting the search bar to 100 characters fixes the search problem, but it has the potential to break our other features as those might be parsing URLs that are longer than 100 characters. This URL was shared as an example.

https://invidious.einfachzocken.eu/watch?v=WLP_L7Mgz6M&index=1&list=PLOJU8YJjFwGOR740NdzTui2NxU-R96YW4

I would suggest taking a step back and digging through the search bar functionality in order to fix the core issue. We'll have to revert the limit in the search bar and only make changes to the functionality that handles search. I would instead show a toast message that states that the query is too long and refuse to handle the search.

If this is something that you're able to do in the next day or so, then feel free to make those changes to this PR. If you don't think you'll have time to make those changes any time soon, then please close this PR and create a new one once you have had time to work on this.

Thank you!

@somanath-goudar
Copy link
Author

Hello @PrestonN,

I now have a better understanding of the issue. I'm committed to resolving it. I'll work on finding a solution over the next 24 hours, and if I'm successful, I'll make the necessary commits to this pull request. However, if I encounter any challenges during this period, I may need to temporarily close the pull request while I investigate the problem further.

@somanath-goudar somanath-goudar marked this pull request as draft November 8, 2023 02:58
auto-merge was automatically disabled November 8, 2023 02:58

Pull request was converted to draft

@github-actions github-actions bot removed the PR: waiting for review For PRs that are complete, tested, and ready for review label Nov 8, 2023
@absidue absidue mentioned this pull request May 22, 2024
1 task
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.

Search Box - Long query

3 participants