Skip to content

fix(url-preview): guard against undefined query string in YouTube URL parser#584

Merged
7w1 merged 1 commit intoSableClient:devfrom
Just-Insane:fix/youtube-url-crash
Mar 30, 2026
Merged

fix(url-preview): guard against undefined query string in YouTube URL parser#584
7w1 merged 1 commit intoSableClient:devfrom
Just-Insane:fix/youtube-url-crash

Conversation

@Just-Insane
Copy link
Copy Markdown
Contributor

Description

Fixes a crash when any non-video YouTube URL is previewed — channels (youtube.com/@handle), channel IDs (youtube.com/channel/...), or any URL path without a ? query string.

The existing code (even after #578) had:

params = path.split('?')[1].split('&');

When there is no ? in the URL, split('?')[1] returns undefined, and calling .split('&') on it throws:

TypeError: Cannot read properties of undefined (reading 'split')

Fixed with optional chaining + nullish fallback:

params = path.split('?')[1]?.split('&') ?? [];

This causes videoId to remain undefined for non-video URLs, which is handled correctly by the existing if (!videoId) return null guard — the preview simply doesn't render rather than crashing the entire app.

Fixes #

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings

AI disclosure:

  • Partially AI assisted (clarify which code was AI assisted and briefly explain what it does).

AI identified the root cause after reviewing the PR #578 diff — the shorts fix added the /shorts/ branch but left the original youtube.com branch with the same unguarded .split() call. The one-line fix (?.split('&') ?? []) was AI-suggested; I reviewed and confirmed it is correct.

@Just-Insane Just-Insane requested review from 7w1 and hazre as code owners March 29, 2026 02:35
Copilot AI review requested due to automatic review settings March 29, 2026 02:35

This comment was marked as spam.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@Just-Insane Just-Insane force-pushed the fix/youtube-url-crash branch from 63e663f to 647509b Compare March 29, 2026 18:03
@7w1
Copy link
Copy Markdown
Member

7w1 commented Mar 29, 2026

looks like your pr accidentally reverts dae8e6a

@7w1 7w1 force-pushed the fix/youtube-url-crash branch from 647509b to 4a110ea Compare March 30, 2026 03:29
@7w1 7w1 enabled auto-merge March 30, 2026 03:31
@7w1 7w1 added this pull request to the merge queue Mar 30, 2026
Merged via the queue into SableClient:dev with commit f60bd22 Mar 30, 2026
12 checks passed
github-merge-queue bot pushed a commit that referenced this pull request Mar 30, 2026
> [!IMPORTANT]
> Merging this PR will create a new release.

## Fixes

* Add youtube shorts support to stop it from crashing sable.
([#578](#578) by @nushea)
* Fix rich-text reply previews and custom-formatted messages so unsafe
HTML is filtered more strictly and Matrix colors render correctly.
([#571](#571) by @hazre)
* Fix crash when previewing non-video YouTube URLs (channels, @Handles,
etc.) that lack query parameters.
([#584](#584) by @Just-Insane)
* fix id handling and id generation for Personas
([#583](#583) by @dozro)
@Just-Insane Just-Insane deleted the fix/youtube-url-crash branch March 30, 2026 12:47
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