-
Notifications
You must be signed in to change notification settings - Fork 3.5k
fix(telegram): allow plain URLs in photo/video/audio/animation fields #3797
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
base: staging
Are you sure you want to change the base?
Changes from all commits
b7e377e
da46a38
fdca736
15ace5e
67aa4bb
34d92fa
115f04e
0d86ea0
af59234
67f8a68
4fd0989
0d2e6ff
e07e3c3
f1ec5fe
70c36cb
3ce9475
6586c5c
8c0a2e0
ecd3536
1c2c2c6
36612ae
e9bdc57
4c12914
84d6fdc
4f3bc37
4bd0731
30f2d1a
ff7b5b5
9fcd02f
1731a4d
19442f1
c78c870
ed9a71f
7b572f1
6bebbc5
ca87d7c
aa0060e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,103 @@ | ||
| import { describe, expect, it } from 'vitest' | ||
| import { normalizeFileInput } from './utils' | ||
|
|
||
| describe('normalizeFileInput', () => { | ||
| describe('URL string handling', () => { | ||
| it('should return URL string as-is for single option', () => { | ||
| const url = 'https://example.com/photo.jpg' | ||
| const result = normalizeFileInput(url, { single: true }) | ||
| expect(result).toBe(url) | ||
| }) | ||
|
|
||
| it('should return URL in array for non-single option', () => { | ||
| const url = 'https://example.com/photo.jpg' | ||
| const result = normalizeFileInput(url, { single: false }) | ||
| expect(result).toEqual([url]) | ||
| }) | ||
|
|
||
| it('should handle HTTP URLs', () => { | ||
| const url = 'http://example.com/photo.jpg' | ||
| const result = normalizeFileInput(url, { single: true }) | ||
| expect(result).toBe(url) | ||
| }) | ||
|
|
||
| it('should trim whitespace from URLs', () => { | ||
| const url = ' https://example.com/photo.jpg ' | ||
| const result = normalizeFileInput(url, { single: true }) | ||
| expect(result).toBe('https://example.com/photo.jpg') | ||
| }) | ||
|
|
||
| it('should return undefined for non-URL strings that fail JSON parse', () => { | ||
| const notAUrl = 'just some text' | ||
| const result = normalizeFileInput(notAUrl, { single: true }) | ||
| expect(result).toBeUndefined() | ||
| }) | ||
| }) | ||
|
|
||
| describe('JSON string handling', () => { | ||
| it('should parse JSON string to object for single option', () => { | ||
| const fileObj = { name: 'test.jpg', url: 'https://example.com/test.jpg' } | ||
| const result = normalizeFileInput(JSON.stringify(fileObj), { single: true }) | ||
| expect(result).toEqual(fileObj) | ||
| }) | ||
|
|
||
| it('should parse JSON string array for non-single option', () => { | ||
| const fileArray = [ | ||
| { name: 'test1.jpg', url: 'https://example.com/test1.jpg' }, | ||
| { name: 'test2.jpg', url: 'https://example.com/test2.jpg' }, | ||
| ] | ||
| const result = normalizeFileInput(JSON.stringify(fileArray), { single: false }) | ||
| expect(result).toEqual(fileArray) | ||
| }) | ||
| }) | ||
|
|
||
| describe('Object and array handling', () => { | ||
| it('should return single object wrapped in array for non-single option', () => { | ||
| const fileObj = { name: 'test.jpg', url: 'https://example.com/test.jpg' } | ||
| const result = normalizeFileInput(fileObj, { single: false }) | ||
| expect(result).toEqual([fileObj]) | ||
| }) | ||
|
|
||
| it('should return single object as-is for single option', () => { | ||
| const fileObj = { name: 'test.jpg', url: 'https://example.com/test.jpg' } | ||
| const result = normalizeFileInput(fileObj, { single: true }) | ||
| expect(result).toEqual(fileObj) | ||
| }) | ||
|
|
||
| it('should return array as-is for non-single option', () => { | ||
| const fileArray = [ | ||
| { name: 'test1.jpg', url: 'https://example.com/test1.jpg' }, | ||
| { name: 'test2.jpg', url: 'https://example.com/test2.jpg' }, | ||
| ] | ||
| const result = normalizeFileInput(fileArray, { single: false }) | ||
| expect(result).toEqual(fileArray) | ||
| }) | ||
| }) | ||
|
|
||
| describe('Edge cases', () => { | ||
| it('should return undefined for null', () => { | ||
| const result = normalizeFileInput(null, { single: true }) | ||
| expect(result).toBeUndefined() | ||
| }) | ||
|
|
||
| it('should return undefined for undefined', () => { | ||
| const result = normalizeFileInput(undefined, { single: true }) | ||
| expect(result).toBeUndefined() | ||
| }) | ||
|
|
||
| it('should return undefined for empty string', () => { | ||
| const result = normalizeFileInput('', { single: true }) | ||
| expect(result).toBeUndefined() | ||
| }) | ||
|
|
||
| it('should throw error for multiple files when single is true', () => { | ||
| const fileArray = [ | ||
| { name: 'test1.jpg', url: 'https://example.com/test1.jpg' }, | ||
| { name: 'test2.jpg', url: 'https://example.com/test2.jpg' }, | ||
| ] | ||
| expect(() => normalizeFileInput(fileArray, { single: true })).toThrow( | ||
| 'File reference must be a single file' | ||
| ) | ||
| }) | ||
| }) | ||
| }) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -326,18 +326,24 @@ const DEFAULT_MULTIPLE_FILES_ERROR = | |
| export function normalizeFileInput( | ||
| fileParam: unknown, | ||
| options: { single: true; errorMessage?: string } | ||
| ): object | undefined | ||
| ): object | string | undefined | ||
| export function normalizeFileInput( | ||
| fileParam: unknown, | ||
| options?: { single?: false } | ||
| ): object[] | undefined | ||
| ): object[] | string | undefined | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Non-single overload return type doesn't match actual returnMedium Severity When Additional Locations (1) |
||
| export function normalizeFileInput( | ||
| fileParam: unknown, | ||
| options?: { single?: boolean; errorMessage?: string } | ||
| ): object | object[] | undefined { | ||
| ): object | object[] | string | undefined { | ||
| if (!fileParam) return undefined | ||
|
|
||
| if (typeof fileParam === 'string') { | ||
| // Check if it's a plain URL (http/https) - return as-is for tools that accept URLs | ||
| const trimmed = fileParam.trim() | ||
| if (trimmed.startsWith('http://') || trimmed.startsWith('https://')) { | ||
| return options?.single ? trimmed : [trimmed] | ||
| } | ||
|
Comment on lines
+342
to
+345
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This change is correct for Telegram media blocks, but For upload-oriented blocks (e.g. Box's Consider scoping this change to Telegram-only, for example by adding an export function normalizeFileInput(
fileParam: unknown,
options: { single: true; allowUrl?: boolean; errorMessage?: string }
): object | string | undefined |
||
|
|
||
| try { | ||
| fileParam = JSON.parse(fileParam) | ||
| } catch { | ||
|
|
||


There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
string[]When a URL is passed with
single: false(or no options), the implementation returns[trimmed]— astring[]. However, the non-single overload declaresobject[] | string | undefinedas its return type. TypeScript doesn't flag this becausestring[]is assignable to the implementation'sobjectvariant (arrays are objects), but it means callers receive a typed promise ofobject[]while actually gettingstring[]at runtime.Any caller that iterates over the returned array and accesses file-object properties (
.name,.url, etc.) would silently getundefinedinstead of failing loudly with a type error. Thetelegram_send_documentpath (normalizeFileInput(params.files)) is one such non-single caller — a URL string passed asparams.fileswould return[url]typed asobject[] | string | undefined, which could confuse downstream handlers.The overload should reflect the actual runtime shape: