diff --git a/app/web/api/v1/create_feed.rb b/app/web/api/v1/create_feed.rb index 5e1fe797..ec96a76a 100644 --- a/app/web/api/v1/create_feed.rb +++ b/app/web/api/v1/create_feed.rb @@ -61,9 +61,11 @@ def build_create_params(params, account) # @param account [Hash] # @return [String] def validated_url(raw_url, account) - url = raw_url.to_s.strip + url = normalized_input_url(raw_url) raise Html2rss::Web::BadRequestError, 'URL parameter is required' if url.empty? - raise Html2rss::Web::BadRequestError, 'Invalid URL format' unless UrlValidator.valid_url?(url) + + url = UrlValidator.canonical_url(url) + raise Html2rss::Web::BadRequestError, 'Invalid URL format' unless url unless UrlValidator.url_allowed?(account, url) raise Html2rss::Web::ForbiddenError, 'URL not allowed for this account' end @@ -71,6 +73,34 @@ def validated_url(raw_url, account) url end + # @param raw_url [String, nil] + # @return [String] + def normalized_input_url(raw_url) + url = raw_url.to_s.strip + return url if url.empty? + return "https:#{url}" if url.start_with?('//') + return url if absolute_url?(url) + + hostname_input?(url) ? "https://#{url}" : url + end + + # @param url [String] + # @return [Boolean] + def absolute_url?(url) + url.match?(%r{\A[a-z][a-z0-9+\-.]*://}i) + end + + # @param url [String] + # @return [Boolean] + def hostname_input?(url) + %r{ + \A + (localhost(?::\d+)?|(?:\d{1,3}\.){3}\d{1,3}(?::\d+)?|(?:[a-z0-9-]+\.)+[a-z]{2,}(?::\d+)?) + (?:[/?#].*)? + \z + }ix.match?(url) + end + # @param raw_strategy [String, nil] # @return [String] def normalize_strategy(raw_strategy) diff --git a/app/web/security/url_validator.rb b/app/web/security/url_validator.rb index 797d26d2..75aacb9e 100644 --- a/app/web/security/url_validator.rb +++ b/app/web/security/url_validator.rb @@ -10,10 +10,16 @@ module UrlValidator MAX_URL_LENGTH = 2048 class << self + # @param url [String] + # @return [String, nil] + def canonical_url(url) + normalize_url(url) + end + # @param url [String] # @return [Boolean] def valid_url?(url) - !normalize_url(url).nil? + !canonical_url(url).nil? end # @param account [Hash] @@ -23,7 +29,7 @@ def url_allowed?(account, url) return false unless account && url allowed_urls = Array(account[:allowed_urls]) - return false unless (normalized_url = normalize_url(url)) + return false unless (normalized_url = canonical_url(url)) return false if allowed_urls.empty? @@ -37,7 +43,7 @@ def url_allowed?(account, url) def match_exact?(pattern, normalized_url) return true if pattern == normalized_url - normalized_pattern = normalize_url(pattern) + normalized_pattern = canonical_url(pattern) normalized_pattern ? normalized_pattern == normalized_url : false end diff --git a/frontend/src/__tests__/App.contract.test.tsx b/frontend/src/__tests__/App.contract.test.tsx index e5375a46..8a3f7e5c 100644 --- a/frontend/src/__tests__/App.contract.test.tsx +++ b/frontend/src/__tests__/App.contract.test.tsx @@ -8,7 +8,7 @@ describe('App contract', () => { const token = 'contract-token'; const authenticate = () => { - window.sessionStorage.setItem('html2rss_access_token', token); + window.localStorage.setItem('html2rss_access_token', token); }; it('shows feed result when API responds with success', async () => { @@ -18,7 +18,7 @@ describe('App contract', () => { http.post('/api/v1/feeds', async ({ request }) => { const body = (await request.json()) as { url: string; strategy: string }; - expect(body).toEqual({ url: 'https://example.com/articles', strategy: 'browserless' }); + expect(body).toEqual({ url: 'https://example.com/articles', strategy: 'faraday' }); expect(request.headers.get('authorization')).toBe(`Bearer ${token}`); return HttpResponse.json( @@ -55,7 +55,7 @@ describe('App contract', () => { await screen.findByLabelText('Page URL'); await waitFor(() => { - expect(screen.getByRole('combobox')).toHaveValue('browserless'); + expect(screen.getByRole('combobox')).toHaveValue('faraday'); }); const urlInput = screen.getByLabelText('Page URL') as HTMLInputElement; @@ -149,7 +149,7 @@ describe('App contract', () => { await screen.findByLabelText('Page URL'); await waitFor(() => { - expect(screen.getByRole('combobox')).toHaveValue('browserless'); + expect(screen.getByRole('combobox')).toHaveValue('faraday'); }); fireEvent.input(screen.getByLabelText('Page URL'), { @@ -161,6 +161,6 @@ describe('App contract', () => { expect(screen.getByText('Add access token')).toBeInTheDocument(); expect(screen.queryByText('Feed generation failed')).not.toBeInTheDocument(); - expect(window.sessionStorage.getItem('html2rss_access_token')).toBeNull(); + expect(window.localStorage.getItem('html2rss_access_token')).toBeNull(); }); }); diff --git a/frontend/src/__tests__/App.test.tsx b/frontend/src/__tests__/App.test.tsx index 3eabb588..5d935469 100644 --- a/frontend/src/__tests__/App.test.tsx +++ b/frontend/src/__tests__/App.test.tsx @@ -91,11 +91,22 @@ describe('App', () => { render(); expect(screen.getByLabelText('html2rss')).toBeInTheDocument(); + expect(screen.getByRole('link', { name: 'html2rss' })).toHaveAttribute('href', '/'); expect(screen.getByLabelText('Page URL')).toBeInTheDocument(); expect(screen.getByRole('button', { name: 'More' })).toBeInTheDocument(); expect(screen.queryByRole('link', { name: 'Bookmarklet' })).not.toBeInTheDocument(); }); + it('keeps the page url field permissive enough for hostname-only input', () => { + render(); + + const urlInput = screen.getByLabelText('Page URL'); + + expect(urlInput).toHaveAttribute('type', 'text'); + expect(urlInput).toHaveAttribute('inputmode', 'url'); + expect(urlInput).toHaveAttribute('autocapitalize', 'off'); + }); + it('autofocuses the source url field', async () => { render(); @@ -104,11 +115,11 @@ describe('App', () => { }); }); - it('prefers browserless as the default strategy when available', () => { + it('prefers faraday as the default strategy when available', () => { render(); return waitFor(() => { - expect(screen.getByRole('combobox')).toHaveValue('browserless'); + expect(screen.getByRole('combobox')).toHaveValue('faraday'); }); }); @@ -140,11 +151,7 @@ describe('App', () => { render(); await waitFor(() => { - expect(mockConvertFeed).toHaveBeenCalledWith( - 'https://example.com/articles', - 'browserless', - 'saved-token' - ); + expect(mockConvertFeed).toHaveBeenCalledWith('https://example.com/articles', 'faraday', 'saved-token'); }); }); @@ -221,7 +228,9 @@ describe('App', () => { preview: { items: [], error: 'Preview unavailable right now.', + isLoading: false, }, + retry: null, }, error: null, convertFeed: mockConvertFeed, @@ -243,6 +252,7 @@ describe('App', () => { result: null, error: 'Access denied', convertFeed: mockConvertFeed, + clearError: mockClearConversionError, clearResult: mockClearResult, }); @@ -331,11 +341,7 @@ describe('App', () => { await waitFor(() => { expect(mockSaveToken).toHaveBeenCalledWith('token-123'); - expect(mockConvertFeed).toHaveBeenCalledWith( - 'https://example.com/articles', - 'browserless', - 'token-123' - ); + expect(mockConvertFeed).toHaveBeenCalledWith('https://example.com/articles', 'faraday', 'token-123'); }); }); @@ -419,13 +425,87 @@ describe('App', () => { expect(bookmarklet.getAttribute('href')).not.toContain('%27+encodeURIComponent'); }); + it('opens token entry immediately for bookmarklet urls when no token is saved', async () => { + window.history.replaceState({}, '', 'http://localhost:3000/?url=example.com%2Farticles'); + + render(); + + await screen.findByText('Add access token'); + expect(screen.getByLabelText('Page URL')).toHaveValue('https://example.com/articles'); + expect(mockConvertFeed).not.toHaveBeenCalled(); + }); + + it('offers a direct alternate strategy retry after conversion failure', async () => { + mockUseAccessToken.mockReturnValue({ + token: 'saved-token', + hasToken: true, + saveToken: mockSaveToken, + clearToken: mockClearToken, + isLoading: false, + error: null, + }); + mockConvertFeed + .mockRejectedValueOnce( + Object.assign(new Error('Tried faraday first, then browserless. Browserless failed.'), { + manualRetryStrategy: 'browserless', + }) + ) + .mockResolvedValueOnce(undefined); + + render(); + + fireEvent.input(screen.getByLabelText('Page URL'), { + target: { value: 'https://example.com/articles' }, + }); + fireEvent.click(screen.getByRole('button', { name: 'Generate feed URL' })); + + await screen.findByRole('button', { name: 'Try browserless instead' }); + fireEvent.click(screen.getByRole('button', { name: 'Try browserless instead' })); + + await waitFor(() => { + expect(mockConvertFeed).toHaveBeenLastCalledWith( + 'https://example.com/articles', + 'browserless', + 'saved-token' + ); + }); + }); + + it('does not offer a duplicate retry action after automatic fallback already failed', async () => { + mockUseAccessToken.mockReturnValue({ + token: 'saved-token', + hasToken: true, + saveToken: mockSaveToken, + clearToken: mockClearToken, + isLoading: false, + error: null, + }); + mockConvertFeed.mockRejectedValueOnce( + Object.assign(new Error('Tried faraday first, then browserless. Browserless failed.'), { + manualRetryStrategy: '', + }) + ); + + render(); + + fireEvent.input(screen.getByLabelText('Page URL'), { + target: { value: 'https://example.com/articles' }, + }); + fireEvent.click(screen.getByRole('button', { name: 'Generate feed URL' })); + + await screen.findByText('Tried faraday first, then browserless. Browserless failed.'); + expect(screen.queryByRole('button', { name: /Try .* instead/ })).not.toBeInTheDocument(); + }); + it('shows the utility links in a user-focused order', () => { window.history.replaceState({}, '', 'http://localhost:3000/#result'); render(); fireEvent.click(screen.getByRole('button', { name: 'More' })); - const utilityLinks = screen.getAllByRole('link').map((link) => link.textContent); + const utilityLinks = Array.from( + screen.getByLabelText('Utilities').querySelectorAll('.utility-strip__items > a') + ).map((link) => link.textContent); expect(utilityLinks).toEqual([ 'Try included feeds', 'Bookmarklet', diff --git a/frontend/src/__tests__/ResultDisplay.test.tsx b/frontend/src/__tests__/ResultDisplay.test.tsx index 696dfb50..818ee0f0 100644 --- a/frontend/src/__tests__/ResultDisplay.test.tsx +++ b/frontend/src/__tests__/ResultDisplay.test.tsx @@ -36,7 +36,9 @@ describe('ResultDisplay', () => { }, ], error: null, + isLoading: false, }, + retry: null, }; beforeEach(() => { @@ -49,6 +51,10 @@ describe('ResultDisplay', () => { expect(screen.getByText('Your feed is ready')).toBeInTheDocument(); expect(screen.getByText('Test Feed')).toBeInTheDocument(); expect(screen.getByRole('button', { name: 'Copy feed URL' })).toBeInTheDocument(); + expect(screen.getByRole('link', { name: 'Subscribe in reader' })).toHaveAttribute( + 'href', + 'feed:https://example.com/feed.xml' + ); expect(screen.getByRole('link', { name: 'Open feed' })).toBeInTheDocument(); expect(screen.getByRole('link', { name: 'Open JSON Feed' })).toHaveAttribute( 'href', @@ -67,7 +73,10 @@ describe('ResultDisplay', () => { it('surfaces preview failures as a result-state message', async () => { render( ); @@ -78,6 +87,39 @@ describe('ResultDisplay', () => { }); }); + it('keeps the result state visible while preview is still loading', async () => { + render( + + ); + + await waitFor(() => { + expect(screen.getByText('Your feed is ready')).toBeInTheDocument(); + expect(screen.getByRole('link', { name: 'Open feed' })).toBeInTheDocument(); + expect(screen.getByText('Loading preview…')).toBeInTheDocument(); + }); + }); + + it('shows an automatic retry notice when fallback strategy succeeded', async () => { + render( + + ); + + await waitFor(() => { + expect( + screen.getByText('Retried automatically with browserless after faraday could not finish the page.') + ).toBeInTheDocument(); + }); + }); + it('calls onCreateAnother when the reset button is clicked', () => { render(); diff --git a/frontend/src/__tests__/useAccessToken.test.ts b/frontend/src/__tests__/useAccessToken.test.ts new file mode 100644 index 00000000..cabf4d63 --- /dev/null +++ b/frontend/src/__tests__/useAccessToken.test.ts @@ -0,0 +1,61 @@ +import { describe, it, expect, beforeEach } from 'vitest'; +import { renderHook, act } from '@testing-library/preact'; +import { useAccessToken } from '../hooks/useAccessToken'; + +describe('useAccessToken', () => { + beforeEach(() => { + window.localStorage.clear(); + window.sessionStorage.clear(); + }); + + it('loads the persisted token from localStorage', async () => { + window.localStorage.setItem('html2rss_access_token', 'persisted-token'); + + const { result } = renderHook(() => useAccessToken()); + + expect(result.current.isLoading).toBe(false); + expect(result.current.token).toBe('persisted-token'); + expect(result.current.hasToken).toBe(true); + expect(result.current.error).toBeNull(); + }); + + it('migrates a legacy session token into localStorage', async () => { + window.sessionStorage.setItem('html2rss_access_token', 'legacy-token'); + + const { result } = renderHook(() => useAccessToken()); + + expect(result.current.isLoading).toBe(false); + expect(result.current.token).toBe('legacy-token'); + expect(window.localStorage.getItem('html2rss_access_token')).toBe('legacy-token'); + expect(window.sessionStorage.getItem('html2rss_access_token')).toBeNull(); + }); + + it('saves new tokens to the persistent storage path', async () => { + const { result } = renderHook(() => useAccessToken()); + + await act(async () => { + await result.current.saveToken('new-token'); + }); + + expect(result.current.token).toBe('new-token'); + expect(result.current.hasToken).toBe(true); + expect(window.localStorage.getItem('html2rss_access_token')).toBe('new-token'); + expect(window.sessionStorage.getItem('html2rss_access_token')).toBeNull(); + }); + + it('clears both persistent and legacy token copies', async () => { + window.localStorage.setItem('html2rss_access_token', 'persisted-token'); + window.sessionStorage.setItem('html2rss_access_token', 'legacy-token'); + + const { result } = renderHook(() => useAccessToken()); + + act(() => { + result.current.clearToken(); + }); + + expect(result.current.token).toBeNull(); + expect(result.current.hasToken).toBe(false); + expect(window.localStorage.getItem('html2rss_access_token')).toBeNull(); + expect(window.sessionStorage.getItem('html2rss_access_token')).toBeNull(); + }); +}); diff --git a/frontend/src/__tests__/useAuth.test.ts b/frontend/src/__tests__/useAuth.test.ts index bb5fd1ba..5995ff27 100644 --- a/frontend/src/__tests__/useAuth.test.ts +++ b/frontend/src/__tests__/useAuth.test.ts @@ -20,11 +20,18 @@ const createStorageMock = (): MockedStorage => { } as unknown as MockedStorage; }; +let localStorageMock: MockedStorage; let sessionStorageMock: MockedStorage; describe('useAuth', () => { beforeEach(() => { + localStorageMock = createStorageMock(); sessionStorageMock = createStorageMock(); + Object.defineProperty(window, 'localStorage', { + value: localStorageMock, + configurable: true, + writable: true, + }); Object.defineProperty(window, 'sessionStorage', { value: sessionStorageMock, configurable: true, @@ -34,7 +41,7 @@ describe('useAuth', () => { }); it('should initialize with unauthenticated state', () => { - sessionStorageMock.getItem.mockReturnValue(null); + localStorageMock.getItem.mockReturnValue(null); const { result } = renderHook(() => useAuth()); @@ -44,7 +51,7 @@ describe('useAuth', () => { }); it('should load auth state from sessionStorage on mount', () => { - sessionStorageMock.getItem + localStorageMock.getItem .mockReturnValueOnce('testuser') // username .mockReturnValueOnce('testtoken'); // token @@ -53,12 +60,12 @@ describe('useAuth', () => { expect(result.current.isAuthenticated).toBe(true); expect(result.current.username).toBe('testuser'); expect(result.current.token).toBe('testtoken'); - expect(sessionStorageMock.getItem).toHaveBeenCalledWith('html2rss_username'); - expect(sessionStorageMock.getItem).toHaveBeenCalledWith('html2rss_token'); + expect(localStorageMock.getItem).toHaveBeenCalledWith('html2rss_username'); + expect(localStorageMock.getItem).toHaveBeenCalledWith('html2rss_token'); }); it('should login and store credentials', async () => { - sessionStorageMock.getItem.mockReturnValue(null); + localStorageMock.getItem.mockReturnValue(null); const { result } = renderHook(() => useAuth()); @@ -69,12 +76,12 @@ describe('useAuth', () => { expect(result.current.isAuthenticated).toBe(true); expect(result.current.username).toBe('newuser'); expect(result.current.token).toBe('newtoken'); - expect(sessionStorageMock.setItem).toHaveBeenCalledWith('html2rss_username', 'newuser'); - expect(sessionStorageMock.setItem).toHaveBeenCalledWith('html2rss_token', 'newtoken'); + expect(localStorageMock.setItem).toHaveBeenCalledWith('html2rss_username', 'newuser'); + expect(localStorageMock.setItem).toHaveBeenCalledWith('html2rss_token', 'newtoken'); }); it('should logout and clear credentials', () => { - sessionStorageMock.getItem.mockReturnValueOnce('testuser').mockReturnValueOnce('testtoken'); + localStorageMock.getItem.mockReturnValueOnce('testuser').mockReturnValueOnce('testtoken'); const { result } = renderHook(() => useAuth()); @@ -85,7 +92,7 @@ describe('useAuth', () => { expect(result.current.isAuthenticated).toBe(false); expect(result.current.username).toBeNull(); expect(result.current.token).toBeNull(); - expect(sessionStorageMock.removeItem).toHaveBeenCalledWith('html2rss_username'); - expect(sessionStorageMock.removeItem).toHaveBeenCalledWith('html2rss_token'); + expect(localStorageMock.removeItem).toHaveBeenCalledWith('html2rss_username'); + expect(localStorageMock.removeItem).toHaveBeenCalledWith('html2rss_token'); }); }); diff --git a/frontend/src/__tests__/useFeedConversion.contract.test.ts b/frontend/src/__tests__/useFeedConversion.contract.test.ts index 737a1572..33cb19dc 100644 --- a/frontend/src/__tests__/useFeedConversion.contract.test.ts +++ b/frontend/src/__tests__/useFeedConversion.contract.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect } from 'vitest'; -import { renderHook, act } from '@testing-library/preact'; +import { renderHook, act, waitFor } from '@testing-library/preact'; import { http, HttpResponse } from 'msw'; import { server, buildFeedResponse } from './mocks/server'; import { useFeedConversion } from '../hooks/useFeedConversion'; @@ -52,8 +52,11 @@ describe('useFeedConversion contract', () => { expect(result.current.result?.feed.feed_token).toBe('generated-token'); expect(result.current.result?.feed.public_url).toBe('/api/v1/feeds/generated-token'); expect(result.current.result?.feed.json_public_url).toBe('/api/v1/feeds/generated-token.json'); - expect(result.current.result?.preview.error).toBeNull(); - expect(result.current.result?.preview.items).toHaveLength(1); + await waitFor(() => { + expect(result.current.result?.preview.error).toBeNull(); + expect(result.current.result?.preview.isLoading).toBe(false); + expect(result.current.result?.preview.items).toHaveLength(1); + }); }); it('propagates API validation errors', async () => { @@ -123,7 +126,10 @@ describe('useFeedConversion contract', () => { expect(result.current.error).toBeNull(); expect(result.current.result?.feed.feed_token).toBe('generated-token'); - expect(result.current.result?.preview.items).toEqual([]); - expect(result.current.result?.preview.error).toBe('Preview unavailable right now.'); + await waitFor(() => { + expect(result.current.result?.preview.items).toEqual([]); + expect(result.current.result?.preview.error).toBe('Preview unavailable right now.'); + expect(result.current.result?.preview.isLoading).toBe(false); + }); }); }); diff --git a/frontend/src/__tests__/useFeedConversion.test.ts b/frontend/src/__tests__/useFeedConversion.test.ts index e58713e3..07d86c27 100644 --- a/frontend/src/__tests__/useFeedConversion.test.ts +++ b/frontend/src/__tests__/useFeedConversion.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect, beforeEach, afterEach, vi, type SpyInstance } from 'vitest'; -import { renderHook, act } from '@testing-library/preact'; +import { renderHook, act, waitFor } from '@testing-library/preact'; import { useFeedConversion } from '../hooks/useFeedConversion'; describe('useFeedConversion', () => { @@ -67,25 +67,39 @@ describe('useFeedConversion', () => { ); const { result } = renderHook(() => useFeedConversion()); + let conversionResult: Awaited> | undefined; await act(async () => { - await result.current.convertFeed('https://example.com', 'faraday', 'testtoken'); + conversionResult = await result.current.convertFeed('https://example.com', 'faraday', 'testtoken'); }); expect(result.current.isConverting).toBe(false); - expect(result.current.result).toEqual({ + expect(conversionResult).toEqual({ feed: mockFeed, preview: { - items: [ - { - title: 'Preview item', - excerpt: 'Preview excerpt', - publishedLabel: 'Jan 2, 2024', - url: 'https://example.com/item', - }, - ], + items: [], error: null, + isLoading: true, }, + retry: null, + }); + await waitFor(() => { + expect(result.current.result).toEqual({ + feed: mockFeed, + preview: { + items: [ + { + title: 'Preview item', + excerpt: 'Preview excerpt', + publishedLabel: 'Jan 2, 2024', + url: 'https://example.com/item', + }, + ], + error: null, + isLoading: false, + }, + retry: null, + }); }); expect(result.current.error).toBeNull(); expect(fetchMock).toHaveBeenCalledTimes(2); @@ -164,19 +178,285 @@ describe('useFeedConversion', () => { fetchMock.mockResolvedValueOnce(new Response('nope', { status: 502 })); const { result } = renderHook(() => useFeedConversion()); + let conversionResult: Awaited> | undefined; await act(async () => { - await result.current.convertFeed('https://example.com', 'faraday', 'testtoken'); + conversionResult = await result.current.convertFeed('https://example.com', 'faraday', 'testtoken'); }); expect(result.current.isConverting).toBe(false); - expect(result.current.result).toEqual({ + expect(conversionResult).toEqual({ feed: createdFeed, preview: { items: [], - error: 'Preview unavailable right now.', + error: null, + isLoading: true, }, + retry: null, + }); + await waitFor(() => { + expect(result.current.result).toEqual({ + feed: createdFeed, + preview: { + items: [], + error: 'Preview unavailable right now.', + isLoading: false, + }, + retry: null, + }); }); expect(result.current.error).toBeNull(); }); + + it('publishes the result before preview loading finishes', async () => { + const createdFeed = { + id: 'test-id', + name: 'Test Feed', + url: 'https://example.com', + strategy: 'faraday', + feed_token: 'test-token', + public_url: 'https://example.com/feed', + json_public_url: 'https://example.com/feed.json', + created_at: '2024-01-01T00:00:00Z', + updated_at: '2024-01-01T00:00:00Z', + }; + + let resolvePreviewResponse: ((value: Response) => void) | null = null; + const previewResponse = new Promise((resolve) => { + resolvePreviewResponse = resolve; + }); + + fetchMock.mockResolvedValueOnce( + new Response( + JSON.stringify({ + success: true, + data: { feed: createdFeed }, + }), + { + status: 201, + headers: { 'Content-Type': 'application/json' }, + } + ) + ); + fetchMock.mockReturnValueOnce(previewResponse as Promise); + + const { result } = renderHook(() => useFeedConversion()); + + let conversionResult: Awaited> | undefined; + await act(async () => { + conversionResult = await result.current.convertFeed('https://example.com', 'faraday', 'testtoken'); + }); + + expect(conversionResult).toEqual({ + feed: createdFeed, + preview: { + items: [], + error: null, + isLoading: true, + }, + retry: null, + }); + expect(result.current.isConverting).toBe(false); + expect(result.current.result).toEqual(conversionResult); + + resolvePreviewResponse?.( + new Response( + JSON.stringify({ + items: [ + { + title: 'Preview item', + content_text: 'Preview excerpt', + url: 'https://example.com/item', + date_published: '2024-01-02T00:00:00Z', + }, + ], + }), + { + status: 200, + headers: { 'Content-Type': 'application/feed+json' }, + } + ) + ); + + await waitFor(() => { + expect(result.current.result?.preview).toEqual({ + items: [ + { + title: 'Preview item', + excerpt: 'Preview excerpt', + publishedLabel: 'Jan 2, 2024', + url: 'https://example.com/item', + }, + ], + error: null, + isLoading: false, + }); + }); + }); + + it('normalizes hostname-only input before creating a feed', async () => { + const createdFeed = { + id: 'test-id', + name: 'Test Feed', + url: 'https://example.com/articles', + strategy: 'faraday', + feed_token: 'test-token', + public_url: 'https://example.com/feed', + json_public_url: 'https://example.com/feed.json', + created_at: '2024-01-01T00:00:00Z', + updated_at: '2024-01-01T00:00:00Z', + }; + + fetchMock.mockResolvedValueOnce( + new Response( + JSON.stringify({ + success: true, + data: { + feed: createdFeed, + }, + }), + { + status: 201, + headers: { 'Content-Type': 'application/json' }, + } + ) + ); + fetchMock.mockResolvedValueOnce( + new Response(JSON.stringify({ items: [] }), { + status: 200, + headers: { 'Content-Type': 'application/feed+json' }, + }) + ); + + const { result } = renderHook(() => useFeedConversion()); + + await act(async () => { + await result.current.convertFeed('example.com/articles', 'faraday', 'testtoken'); + }); + + const firstRequest = fetchMock.mock.calls[0]?.[0] as Request; + expect(firstRequest instanceof Request ? firstRequest.url : String(firstRequest)).toContain( + '/api/v1/feeds' + ); + expect(await firstRequest.clone().json()).toEqual({ + url: 'https://example.com/articles', + strategy: 'faraday', + }); + }); + + it('automatically retries browserless after a faraday failure', async () => { + const createdFeed = { + id: 'test-id', + name: 'Test Feed', + url: 'https://example.com/articles', + strategy: 'browserless', + feed_token: 'test-token', + public_url: 'https://example.com/feed', + json_public_url: 'https://example.com/feed.json', + created_at: '2024-01-01T00:00:00Z', + updated_at: '2024-01-01T00:00:00Z', + }; + + fetchMock + .mockResolvedValueOnce( + new Response( + JSON.stringify({ + success: false, + error: { message: 'Upstream timeout' }, + }), + { + status: 502, + headers: { 'Content-Type': 'application/json' }, + } + ) + ) + .mockResolvedValueOnce( + new Response( + JSON.stringify({ + success: true, + data: { + feed: createdFeed, + }, + }), + { + status: 201, + headers: { 'Content-Type': 'application/json' }, + } + ) + ) + .mockResolvedValueOnce( + new Response(JSON.stringify({ items: [] }), { + status: 200, + headers: { 'Content-Type': 'application/feed+json' }, + }) + ); + + const { result } = renderHook(() => useFeedConversion()); + + await act(async () => { + await result.current.convertFeed('https://example.com/articles', 'faraday', 'testtoken'); + }); + + const retryRequest = fetchMock.mock.calls[1]?.[0] as Request; + expect(await retryRequest.clone().json()).toEqual({ + url: 'https://example.com/articles', + strategy: 'browserless', + }); + expect(result.current.result?.retry).toEqual({ + automatic: true, + from: 'faraday', + to: 'browserless', + }); + await waitFor(() => { + expect(result.current.result?.preview.isLoading).toBe(false); + }); + }); + + it('does not offer a duplicate manual retry after automatic fallback also fails', async () => { + fetchMock + .mockResolvedValueOnce( + new Response( + JSON.stringify({ + success: false, + error: { message: 'Upstream timeout' }, + }), + { + status: 502, + headers: { 'Content-Type': 'application/json' }, + } + ) + ) + .mockResolvedValueOnce( + new Response( + JSON.stringify({ + success: false, + error: { message: 'Browserless also failed' }, + }), + { + status: 502, + headers: { 'Content-Type': 'application/json' }, + } + ) + ); + + const { result } = renderHook(() => useFeedConversion()); + + let thrownError: (Error & { manualRetryStrategy?: string }) | undefined; + await act(async () => { + try { + await result.current.convertFeed('https://example.com/articles', 'faraday', 'testtoken'); + } catch (error) { + thrownError = error as Error & { manualRetryStrategy?: string }; + } + }); + + expect(thrownError?.message).toBe( + 'Tried faraday first, then browserless. First attempt failed with: Upstream timeout. Second attempt failed with: Browserless also failed' + ); + expect(thrownError?.manualRetryStrategy).toBeUndefined(); + expect(result.current.result).toBeNull(); + expect(result.current.error).toBe( + 'Tried faraday first, then browserless. First attempt failed with: Upstream timeout. Second attempt failed with: Browserless also failed' + ); + }); }); diff --git a/frontend/src/api/contracts.ts b/frontend/src/api/contracts.ts index a012dcea..041154d4 100644 --- a/frontend/src/api/contracts.ts +++ b/frontend/src/api/contracts.ts @@ -12,11 +12,19 @@ export interface FeedPreviewItem { export interface FeedPreviewState { items: FeedPreviewItem[]; error: string | null; + isLoading: boolean; +} + +export interface FeedRetryState { + automatic: boolean; + from: string; + to: string; } export interface CreatedFeedResult { feed: FeedRecord; preview: FeedPreviewState; + retry: FeedRetryState | null; } export interface ApiMetadataRecord { diff --git a/frontend/src/api/generated/types.gen.ts b/frontend/src/api/generated/types.gen.ts index 2855556b..49422cdd 100644 --- a/frontend/src/api/generated/types.gen.ts +++ b/frontend/src/api/generated/types.gen.ts @@ -80,7 +80,7 @@ export type CreateFeedError = CreateFeedErrors[keyof CreateFeedErrors]; export type CreateFeedResponses = { /** - * creates a feed when request is valid + * normalizes hostname-only input to https before feed creation */ 201: { data: { diff --git a/frontend/src/components/App.tsx b/frontend/src/components/App.tsx index 8fa7d018..80e6a009 100644 --- a/frontend/src/components/App.tsx +++ b/frontend/src/components/App.tsx @@ -5,22 +5,27 @@ import { useAccessToken } from '../hooks/useAccessToken'; import { useApiMetadata } from '../hooks/useApiMetadata'; import { useFeedConversion } from '../hooks/useFeedConversion'; import { useStrategies } from '../hooks/useStrategies'; +import { normalizeUserUrl } from '../utils/url'; const EMPTY_FEED_ERRORS = { url: '', form: '' }; const DEFAULT_FEED_CREATION = { enabled: true, access_token_required: true }; const preferredStrategy = (strategies: { id: string }[]) => - strategies.find((strategy) => strategy.id === 'browserless')?.id ?? strategies[0]?.id; + strategies.find((strategy) => strategy.id === 'faraday')?.id ?? strategies[0]?.id; + +interface ConversionErrorWithMeta extends Error { + manualRetryStrategy?: string; +} function BrandLockup() { return ( -
+
+ ); } @@ -49,6 +54,7 @@ export function App() { const [showTokenPrompt, setShowTokenPrompt] = useState(false); const [tokenDraft, setTokenDraft] = useState(''); const [tokenError, setTokenError] = useState(''); + const [manualRetryStrategy, setManualRetryStrategy] = useState(''); const [focusCreateComposerKey, setFocusCreateComposerKey] = useState(0); const autoSubmitUrlRef = useRef(null); const hasAutoSubmittedRef = useRef(false); @@ -84,6 +90,7 @@ export function App() { url: key === 'url' ? '' : prev.url, form: '', })); + setManualRetryStrategy(''); clearError(); }; @@ -103,10 +110,11 @@ export function App() { ); }; - const attemptFeedCreation = async (accessToken: string) => { - const strategy = selectedStrategy; + const attemptFeedCreation = async (accessToken: string, strategyOverride?: string) => { + const strategy = strategyOverride || selectedStrategy; + const normalizedUrl = normalizeUserUrl(feedFormData.url); - if (!feedFormData.url.trim()) { + if (!normalizedUrl) { setFeedFieldErrors({ ...EMPTY_FEED_ERRORS, url: 'Source URL is required.' }); return false; } @@ -125,6 +133,7 @@ export function App() { } if (feedCreation.access_token_required && !accessToken) { + setFeedFormData((prev) => ({ ...prev, url: normalizedUrl })); clearError(); setShowTokenPrompt(true); setTokenError(''); @@ -132,12 +141,16 @@ export function App() { } try { - await convertFeed(feedFormData.url, strategy, accessToken); + setFeedFormData((prev) => ({ ...prev, url: normalizedUrl })); + await convertFeed(normalizedUrl, strategy, accessToken); setShowTokenPrompt(false); setTokenError(''); + setManualRetryStrategy(''); return true; } catch (submitError) { const message = submitError instanceof Error ? submitError.message : 'Unable to start feed generation.'; + const retryStrategy = (submitError as ConversionErrorWithMeta).manualRetryStrategy ?? ''; + setManualRetryStrategy(retryStrategy); if (feedCreation.access_token_required && isAccessTokenError(message)) { clearToken(); @@ -178,19 +191,45 @@ export function App() { const handleCreateAnother = () => { clearResult(); + setManualRetryStrategy(''); setFocusCreateComposerKey((current) => current + 1); }; + const handleRetryWithStrategy = () => { + if (!manualRetryStrategy) return; + + setFeedFormData((prev) => ({ ...prev, strategy: manualRetryStrategy })); + setFeedFieldErrors(EMPTY_FEED_ERRORS); + clearError(); + void attemptFeedCreation(token ?? '', manualRetryStrategy); + }; + useEffect(() => { const autoSubmitUrl = autoSubmitUrlRef.current; if (!autoSubmitUrl || hasAutoSubmittedRef.current) return; if (strategiesLoading || metadataLoading || tokenLoading) return; if (feedFormData.url !== autoSubmitUrl || !selectedStrategy) return; + if (feedCreation.access_token_required && !token) { + hasAutoSubmittedRef.current = true; + setFeedFormData((prev) => ({ ...prev, url: normalizeUserUrl(autoSubmitUrl) })); + setShowTokenPrompt(true); + setTokenError(''); + return; + } + hasAutoSubmittedRef.current = true; setFeedFieldErrors(EMPTY_FEED_ERRORS); void attemptFeedCreation(token ?? ''); - }, [feedFormData.url, metadataLoading, selectedStrategy, strategiesLoading, token, tokenLoading]); + }, [ + feedCreation.access_token_required, + feedFormData.url, + metadataLoading, + selectedStrategy, + strategiesLoading, + token, + tokenLoading, + ]); if (metadataLoading || tokenLoading) { return ( @@ -254,6 +293,8 @@ export function App() { setTokenError(''); clearError(); }} + manualRetryStrategy={manualRetryStrategy} + onRetryWithStrategy={handleRetryWithStrategy} strategyHint={strategyHint} /> void; onFeedFieldChange: (key: 'url' | 'strategy', value: string) => void; onTokenDraftChange: (value: string) => void; onSaveToken: () => void; onCancelTokenPrompt: () => void; + onRetryWithStrategy: () => void; strategyHint: (strategy: Strategy) => string; } @@ -60,11 +62,13 @@ export function CreateFeedPanel({ tokenDraft, tokenError, showTokenPrompt, + manualRetryStrategy, onFeedSubmit, onFeedFieldChange, onTokenDraftChange, onSaveToken, onCancelTokenPrompt, + onRetryWithStrategy, strategyHint, }: CreateFeedPanelProps) { const selectedStrategy = strategies.find((strategy) => strategy.id === feedFormData.strategy); @@ -106,9 +110,12 @@ export function CreateFeedPanel({ { @@ -246,6 +258,13 @@ export function CreateFeedPanel({ {feedFieldErrors.form && ( )} diff --git a/frontend/src/components/Bookmarklet.tsx b/frontend/src/components/Bookmarklet.tsx index a83c8f00..8c839795 100644 --- a/frontend/src/components/Bookmarklet.tsx +++ b/frontend/src/components/Bookmarklet.tsx @@ -2,17 +2,18 @@ export function Bookmarklet() { const bookmarkletHref = (() => { if (typeof window === 'undefined') return '#'; - const appUrl = new URL(window.location.href); - appUrl.search = ''; - appUrl.hash = ''; + const targetPrefix = `${new URL('/', window.location.href).toString()}?url=`; - const targetPrefix = `${appUrl.origin}/?url=`; - - return `javascript:window.location.href=${JSON.stringify(targetPrefix)}+encodeURIComponent(window.location.href);`; + return `javascript:window.location.assign(${JSON.stringify(targetPrefix)}+encodeURIComponent(window.location.href));`; })(); return ( - + Bookmarklet ); diff --git a/frontend/src/components/DominantField.tsx b/frontend/src/components/DominantField.tsx index 73b1a58d..8324c576 100644 --- a/frontend/src/components/DominantField.tsx +++ b/frontend/src/components/DominantField.tsx @@ -7,6 +7,9 @@ interface DominantFieldProps { value: string; placeholder?: string; type?: string; + inputMode?: JSX.HTMLAttributes['inputMode']; + autoCapitalize?: JSX.HTMLAttributes['autoCapitalize']; + spellcheck?: boolean; readOnly?: boolean; autoFocus?: boolean; disabled?: boolean; @@ -26,6 +29,9 @@ export function DominantField({ value, placeholder, type = 'text', + inputMode, + autoCapitalize, + spellcheck, readOnly = false, autoFocus = false, disabled = false, @@ -47,7 +53,10 @@ export function DominantField({ type={type} class="input input--mono input--lg" placeholder={placeholder} - autocomplete={type === 'url' ? 'url' : 'off'} + autoComplete={type === 'url' ? 'url' : 'off'} + inputMode={inputMode} + autoCapitalize={autoCapitalize} + spellcheck={spellcheck} autoFocus={autoFocus} ref={inputRef} value={value} diff --git a/frontend/src/components/ResultDisplay.tsx b/frontend/src/components/ResultDisplay.tsx index 67afaadd..81a6dd36 100644 --- a/frontend/src/components/ResultDisplay.tsx +++ b/frontend/src/components/ResultDisplay.tsx @@ -18,6 +18,7 @@ export function ResultDisplay({ result, onCreateAnother }: ResultDisplayProps) { const jsonFeedUrl = feed.json_public_url.startsWith('http') ? feed.json_public_url : `${window.location.origin}${feed.json_public_url}`; + const subscribeUrl = /^https?:\/\//i.test(fullUrl) ? `feed:${fullUrl}` : null; useEffect(() => { return () => { @@ -46,6 +47,11 @@ export function ResultDisplay({ result, onCreateAnother }: ResultDisplayProps) {

Your feed is ready

{feed.name}

Subscribe to this URL in your RSS reader.

+ {result.retry && ( +

+ {`Retried automatically with ${result.retry.to} after ${result.retry.from} could not finish the page.`} +

+ )}
+ {subscribeUrl && ( + + Subscribe in reader + + )} Open feed @@ -72,6 +83,16 @@ export function ResultDisplay({ result, onCreateAnother }: ResultDisplayProps) {
+ {preview.isLoading && ( +
+
+

Preview

+

Latest items from this feed

+
+

Loading preview…

+
+ )} + {preview.items.length > 0 && (
@@ -99,7 +120,7 @@ export function ResultDisplay({ result, onCreateAnother }: ResultDisplayProps) {
)} - {preview.error && ( + {!preview.isLoading && preview.error && (

Preview

diff --git a/frontend/src/hooks/useAccessToken.ts b/frontend/src/hooks/useAccessToken.ts index 01f022cc..eff39b28 100644 --- a/frontend/src/hooks/useAccessToken.ts +++ b/frontend/src/hooks/useAccessToken.ts @@ -31,9 +31,23 @@ const resolveStorage = (): Storage => { if (typeof window === 'undefined') return memoryStorage; try { - return window.sessionStorage ?? memoryStorage; + return window.localStorage ?? window.sessionStorage ?? memoryStorage; } catch { - return memoryStorage; + try { + return window.sessionStorage ?? memoryStorage; + } catch { + return memoryStorage; + } + } +}; + +const clearLegacySessionToken = () => { + if (typeof window === 'undefined') return; + + try { + window.sessionStorage?.removeItem(ACCESS_TOKEN_KEY); + } catch { + // Ignore restricted sessionStorage access (privacy mode, sandboxed contexts). } }; @@ -49,9 +63,23 @@ export function useAccessToken() { try { const token = storage.getItem(ACCESS_TOKEN_KEY)?.trim() ?? ''; + let legacyToken = ''; + if (!token && typeof window !== 'undefined') { + try { + legacyToken = window.sessionStorage?.getItem(ACCESS_TOKEN_KEY)?.trim() ?? ''; + } catch { + // Treat restricted sessionStorage access as no legacy token. + legacyToken = ''; + } + } + + if (!token && legacyToken) { + storage.setItem(ACCESS_TOKEN_KEY, legacyToken); + clearLegacySessionToken(); + } setState({ - token: token || null, + token: token || legacyToken || null, isLoading: false, error: null, }); @@ -70,6 +98,7 @@ export function useAccessToken() { const storage = resolveStorage(); storage.setItem(ACCESS_TOKEN_KEY, normalized); + clearLegacySessionToken(); setState({ token: normalized, @@ -81,6 +110,7 @@ export function useAccessToken() { const clearToken = () => { const storage = resolveStorage(); storage.removeItem(ACCESS_TOKEN_KEY); + clearLegacySessionToken(); setState({ token: null, diff --git a/frontend/src/hooks/useAuth.ts b/frontend/src/hooks/useAuth.ts index a3e1ddfc..197f50ac 100644 --- a/frontend/src/hooks/useAuth.ts +++ b/frontend/src/hooks/useAuth.ts @@ -38,9 +38,13 @@ const resolveStorage = (): Storage => { } try { - return window.sessionStorage ?? memoryStorage; + return window.localStorage ?? window.sessionStorage ?? memoryStorage; } catch (error) { - return memoryStorage; + try { + return window.sessionStorage ?? memoryStorage; + } catch { + return memoryStorage; + } } }; diff --git a/frontend/src/hooks/useFeedConversion.ts b/frontend/src/hooks/useFeedConversion.ts index aeab5a3e..3ff52d31 100644 --- a/frontend/src/hooks/useFeedConversion.ts +++ b/frontend/src/hooks/useFeedConversion.ts @@ -1,7 +1,8 @@ -import { useState } from 'preact/hooks'; +import { useRef, useState } from 'preact/hooks'; import { createFeed } from '../api/generated'; import { apiClient } from '../api/client'; import type { CreatedFeedResult, FeedPreviewItem, FeedRecord } from '../api/contracts'; +import { normalizeUserUrl } from '../utils/url'; interface JsonFeedItem { title?: string; @@ -22,7 +23,13 @@ interface ConversionState { error: string | null; } +interface ConversionError extends Error { + manualRetryStrategy?: string; + autoRetryAttempted?: boolean; +} + export function useFeedConversion() { + const requestIdRef = useRef(0); const [state, setState] = useState({ isConverting: false, result: null, @@ -30,57 +37,85 @@ export function useFeedConversion() { }); const convertFeed = async (url: string, strategy: string, token: string) => { - if (!url?.trim()) throw new Error('URL is required'); - if (!strategy?.trim()) throw new Error('Strategy is required'); + const normalizedUrl = normalizeUserUrl(url); + const requestedStrategy = strategy.trim(); + const fallbackStrategy = requestedStrategy === 'faraday' ? 'browserless' : null; - try { - new URL(url.trim()); - } catch { + if (!normalizedUrl) throw new Error('URL is required'); + if (!requestedStrategy) throw new Error('Strategy is required'); + + if (!isValidHttpUrl(normalizedUrl)) { throw new Error('Invalid URL format'); } + const requestId = requestIdRef.current + 1; + requestIdRef.current = requestId; setState((prev) => ({ ...prev, isConverting: true, error: null })); try { - const response = await createFeed({ - client: apiClient, - headers: { - Authorization: `Bearer ${token}`, - }, - body: { - url: url.trim(), - strategy: strategy.trim(), - }, - throwOnError: true, - }); + const feed = await requestFeedCreation(normalizedUrl, requestedStrategy, token); + const result = { + feed, + preview: buildLoadingPreviewState(), + retry: null, + }; - if (!response.data?.success || !response.data.data?.feed) { - throw new Error('Invalid response format'); + setState((prev) => ({ ...prev, isConverting: false, result, error: null })); + void hydratePreview(feed, requestId, null, setState, requestIdRef); + return result; + } catch (firstError) { + if (shouldAutoRetry(requestedStrategy, fallbackStrategy, firstError)) { + try { + const feed = await requestFeedCreation(normalizedUrl, fallbackStrategy, token); + const result = { + feed, + preview: buildLoadingPreviewState(), + retry: { automatic: true, from: requestedStrategy, to: fallbackStrategy }, + }; + + setState((prev) => ({ ...prev, isConverting: false, result, error: null })); + void hydratePreview(feed, requestId, result.retry, setState, requestIdRef); + return result; + } catch (secondError) { + const message = buildRetryFailureMessage( + firstError, + secondError, + requestedStrategy, + fallbackStrategy + ); + const retryError = buildConversionError(message, { + manualRetryStrategy: undefined, + autoRetryAttempted: true, + }); + + setState((prev) => ({ + ...prev, + isConverting: false, + error: message, + result: null, + })); + throw retryError; + } } - const feed = response.data.data.feed; - const preview = await loadPreview(feed).catch((error: unknown) => ({ - items: [], - error: toPreviewErrorMessage(error), - })); - const result = { feed, preview }; + const message = toErrorMessage(firstError); + const retryError = buildConversionError(message, { + manualRetryStrategy: alternateStrategy(requestedStrategy), + }); - setState((prev) => ({ ...prev, isConverting: false, result, error: null })); - return result; - } catch (error) { - const message = toErrorMessage(error); setState((prev) => ({ ...prev, isConverting: false, error: message, result: null, })); - throw new Error(message); + throw retryError; } }; const clearResult = () => { window.document.body.scrollIntoView({ behavior: 'smooth', block: 'start' }); + requestIdRef.current += 1; setState({ isConverting: false, @@ -120,9 +155,129 @@ async function loadPreview(feed: FeedRecord): Promise 0 ? null : 'Preview unavailable right now.', + isLoading: false, }; } +function buildLoadingPreviewState(): CreatedFeedResult['preview'] { + return { + items: [], + error: null, + isLoading: true, + }; +} + +async function hydratePreview( + feed: FeedRecord, + requestId: number, + retry: CreatedFeedResult['retry'], + setState: (value: ConversionState | ((prev: ConversionState) => ConversionState)) => void, + requestIdRef: { current: number } +) { + const preview = await loadPreview(feed).catch((error: unknown) => ({ + items: [], + error: toPreviewErrorMessage(error), + isLoading: false, + })); + + if (requestIdRef.current !== requestId) return; + + setState((prev) => { + if ( + requestIdRef.current !== requestId || + !prev.result || + prev.result.feed.feed_token !== feed.feed_token + ) { + return prev; + } + + return { + ...prev, + result: { + feed, + preview, + retry, + }, + }; + }); +} + +async function requestFeedCreation(url: string, strategy: string, token: string): Promise { + const response = await createFeed({ + client: apiClient, + headers: { + Authorization: `Bearer ${token}`, + }, + body: { + url, + strategy, + }, + throwOnError: true, + }); + + if (!response.data?.success || !response.data.data?.feed) { + throw new Error('Invalid response format'); + } + + return response.data.data.feed; +} + +function isValidHttpUrl(value: string): boolean { + try { + const url = new URL(value); + return url.protocol === 'http:' || url.protocol === 'https:'; + } catch { + return false; + } +} + +function alternateStrategy(strategy: string): string | undefined { + if (strategy === 'faraday') return 'browserless'; + if (strategy === 'browserless') return 'faraday'; + return undefined; +} + +function shouldAutoRetry( + strategy: string, + fallbackStrategy: string | null, + error: unknown +): fallbackStrategy is string { + if (strategy !== 'faraday' || !fallbackStrategy) return false; + + const normalized = toErrorMessage(error).toLowerCase(); + return !( + normalized.includes('unauthorized') || + normalized.includes('bad request') || + normalized.includes('forbidden') || + normalized.includes('access token') || + normalized.includes('authentication') || + normalized.includes('invalid response format') || + normalized.includes('network error') || + normalized.includes('url') || + normalized.includes('unsupported strategy') + ); +} + +function buildRetryFailureMessage( + firstError: unknown, + secondError: unknown, + requestedStrategy: string, + fallbackStrategy: string +): string { + const secondMessage = toErrorMessage(secondError); + const firstMessage = toErrorMessage(firstError); + + if (firstMessage === secondMessage) { + return `Tried ${requestedStrategy} first, then ${fallbackStrategy}. ${secondMessage}`; + } + + return `Tried ${requestedStrategy} first, then ${fallbackStrategy}. First attempt failed with: ${firstMessage}. Second attempt failed with: ${secondMessage}`; +} + +function buildConversionError(message: string, metadata: Partial): ConversionError { + return Object.assign(new Error(message), metadata); +} + const toErrorMessage = (error: unknown): string => { if (error instanceof SyntaxError) return 'Invalid response format from feed creation API'; if (error instanceof Error) return error.message; diff --git a/frontend/src/styles/main.css b/frontend/src/styles/main.css index 0389addc..40721669 100644 --- a/frontend/src/styles/main.css +++ b/frontend/src/styles/main.css @@ -616,6 +616,10 @@ a:focus-visible { color: var(--text-muted); } +.notice__actions { + margin-top: var(--space-3); +} + .notice__spinner { width: 1rem; height: 1rem; diff --git a/frontend/src/utils/url.ts b/frontend/src/utils/url.ts new file mode 100644 index 00000000..efb7c784 --- /dev/null +++ b/frontend/src/utils/url.ts @@ -0,0 +1,26 @@ +const SCHEME_PATTERN = /^[a-z][a-z0-9+.-]*:\/\//i; +const HOST_LIKE_PATTERN = + /^(localhost(?::\d+)?|(?:\d{1,3}\.){3}\d{1,3}(?::\d+)?|(?:[a-z0-9-]+\.)+[a-z]{2,}(?::\d+)?)(?:[/?#].*)?$/i; + +export function normalizeUserUrl(rawValue: string): string { + const value = rawValue.trim(); + if (!value) return ''; + + if (value.startsWith('//')) return `https:${value}`; + if (SCHEME_PATTERN.test(value)) return value; + if (HOST_LIKE_PATTERN.test(value)) return `https://${value}`; + + return value; +} + +export function isNormalizedHttpUrl(rawValue: string): boolean { + const normalized = normalizeUserUrl(rawValue); + if (!normalized) return false; + + try { + const url = new URL(normalized); + return url.protocol === 'http:' || url.protocol === 'https:'; + } catch { + return false; + } +} diff --git a/public/openapi.yaml b/public/openapi.yaml index a14637b5..714d08fd 100644 --- a/public/openapi.yaml +++ b/public/openapi.yaml @@ -167,7 +167,7 @@ paths: - data - meta type: object - description: creates a feed when request is valid + description: normalizes hostname-only input to https before feed creation '401': content: application/json: diff --git a/public/shared-ui.css b/public/shared-ui.css index 003588a4..9bb4b48a 100644 --- a/public/shared-ui.css +++ b/public/shared-ui.css @@ -178,6 +178,7 @@ textarea { display: inline-grid; justify-items: center; gap: var(--brand-lockup-gap); + text-decoration: none; } .brand-lockup__mark { diff --git a/spec/html2rss/web/api/v1_spec.rb b/spec/html2rss/web/api/v1_spec.rb index 963f9f50..894ce2f5 100644 --- a/spec/html2rss/web/api/v1_spec.rb +++ b/spec/html2rss/web/api/v1_spec.rb @@ -59,6 +59,12 @@ def valid_feed_token Html2rss::Web::Auth.generate_feed_token('admin', feed_url, strategy: 'faraday') end + def post_feed_request(payload) + header 'Authorization', "Bearer #{admin_token}" + header 'Content-Type', 'application/json' + post '/api/v1/feeds', payload.to_json + end + def json_feed_response_for(token) stub_json_feed_success get "/api/v1/feeds/#{token}", {}, { 'HTTP_ACCEPT' => 'application/feed+json' } @@ -453,6 +459,23 @@ def expected_featured_feeds expect(last_response.headers['Content-Type']).to include('application/json') end + it 'normalizes hostname-only input to https before feed creation', :aggregate_failures do + allow(Html2rss::Web::AutoSource).to receive(:create_stable_feed).and_call_original + + post_feed_request(url: 'example.com/articles', strategy: 'faraday') + + expect(Html2rss::Web::AutoSource).to have_received(:create_stable_feed).with( + anything, + 'https://example.com/articles', + kind_of(Hash), + 'faraday' + ) + + expect(last_response.status).to eq(201) + json = expect_success_response(last_response) + expect(json.dig('data', 'feed', 'url')).to eq('https://example.com/articles') + end + it 'returns forbidden for authenticated requests when auto source is disabled', :aggregate_failures do header 'Authorization', "Bearer #{admin_token}" header 'Content-Type', 'application/json'