Skip to content

Commit 567cd13

Browse files
Copilotllun
Copilot
and
llun
authored
Fix critical bugs in feed parsing, OPML validation, and error handling (#688)
* Fix date parsing, null checks, and OPML validation bugs Co-authored-by: llun <[email protected]> * Fix error handling with proper type annotations and process exit on failure Co-authored-by: llun <[email protected]> * Fix potential null reference error in parseAtom link handling Co-authored-by: llun <[email protected]> * Bump version to 3.1.2 Co-authored-by: llun <[email protected]> * Bump version to 3.1.3 Co-authored-by: llun <[email protected]> --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: llun <[email protected]>
1 parent 4fff1c3 commit 567cd13

File tree

11 files changed

+336
-35
lines changed

11 files changed

+336
-35
lines changed

‎action/feeds/database.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ export function getDatabase(contentDirectory: string) {
1919
if (!stats.isDirectory()) {
2020
throw new Error(`${contentDirectory} is not a directory`)
2121
}
22-
} catch (error) {
22+
} catch (error: any) {
2323
if (error.code !== 'ENOENT') {
2424
throw new Error(`Fail to access ${contentDirectory}`)
2525
}
@@ -131,7 +131,7 @@ export async function insertCategory(knex: Knex, category: string) {
131131
if (record) return
132132
await trx('Categories').insert({ name: category })
133133
})
134-
} catch (error) {
134+
} catch (error: any) {
135135
console.error(`Fail to insert ${category}`)
136136
}
137137
}
@@ -288,7 +288,7 @@ export async function insertSite(knex: Knex, category: string, site: Site) {
288288
return key
289289
})
290290
return key
291-
} catch (error) {
291+
} catch (error: any) {
292292
console.error(`Fail to insert site ${site.title}`)
293293
console.error(error.message)
294294
return null
@@ -407,7 +407,7 @@ export async function copyExistingDatabase(publicPath: string) {
407407
fs.statSync(existingDatabase)
408408
console.log(`Copying ${existingDatabase} to ${targetDatabase}`)
409409
fs.copyFileSync(existingDatabase, targetDatabase, constants.COPYFILE_EXCL)
410-
} catch (error) {
410+
} catch (error: any) {
411411
// Fail to read old database, ignore it
412412
console.log('Skip copy old database because of error: ', error.message)
413413
}

‎action/feeds/file.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ export async function createCategoryDirectory(
6565
`${path.join(rootDirectory, category)} is not a directory`
6666
)
6767
}
68-
} catch (error) {
68+
} catch (error: any) {
6969
if (error.code !== 'ENOENT') {
7070
throw new Error(`Fail to access ${rootDirectory}`)
7171
}

‎action/feeds/index.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ export async function createFeedDatabase(githubActionPath: string) {
3838
await createOrUpdateDatabase(database, opml, loadFeed)
3939
await cleanup(database)
4040
await database.destroy()
41-
} catch (error) {
41+
} catch (error: any) {
4242
console.error(error.message)
4343
console.error(error.stack)
4444
core.setFailed(error)
@@ -65,7 +65,7 @@ export async function createFeedFiles(githubActionPath: string) {
6565
await createRepositoryData(DEFAULT_PATHS, githubRootName, customDomainName)
6666
await createCategoryData(DEFAULT_PATHS)
6767
await createAllEntriesData()
68-
} catch (error) {
68+
} catch (error: any) {
6969
console.error(error.message)
7070
console.error(error.stack)
7171
core.setFailed(error)

‎action/feeds/opml-validation.test.ts‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
import test from 'ava'
2+
import { readOpml } from './opml'
3+
4+
test('#readOpml handles malformed OPML with missing attributes', async (t) => {
5+
const opml = `<?xml version="1.0" encoding="UTF-8"?>
6+
<opml version="2.0">
7+
<body>
8+
<outline title="Category 1">
9+
<outline type="rss" title="Feed 1" xmlUrl="https://example.com/feed1.xml"/>
10+
<outline title="No type attribute" xmlUrl="https://example.com/feed2.xml"/>
11+
</outline>
12+
<outline>
13+
<outline type="rss" title="Feed 3" xmlUrl="https://example.com/feed3.xml"/>
14+
</outline>
15+
</body>
16+
</opml>`
17+
18+
const result = await readOpml(opml)
19+
t.truthy(result)
20+
// Should only include valid feeds and categories
21+
const category1 = result.find(cat => cat.category === 'Category 1')
22+
t.truthy(category1)
23+
t.is(category1?.items.length, 1) // Should only include Feed 1
24+
})
25+
26+
test('#readOpml throws error for invalid OPML structure', async (t) => {
27+
const opml = `<?xml version="1.0" encoding="UTF-8"?>
28+
<opml version="2.0">
29+
<head>
30+
<title>Test</title>
31+
</head>
32+
</opml>`
33+
34+
await t.throwsAsync(
35+
async () => await readOpml(opml),
36+
{ message: /Invalid OPML format/ }
37+
)
38+
})
39+
40+
test('#readOpml throws error for completely empty OPML', async (t) => {
41+
const opml = `<?xml version="1.0" encoding="UTF-8"?>
42+
<opml version="2.0">
43+
</opml>`
44+
45+
await t.throwsAsync(
46+
async () => await readOpml(opml),
47+
{ message: /Invalid OPML format/ }
48+
)
49+
})

‎action/feeds/opml.ts‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ export async function loadFeed(title: string, url: string) {
1313

1414
const site = 'rss' in xml ? parseRss(title, xml) : parseAtom(title, xml)
1515
return site
16-
} catch (error) {
16+
} catch (error: any) {
1717
console.error(
1818
`Fail to load - ${title} (${url}) because of ${error.message}`
1919
)
@@ -35,15 +35,22 @@ export interface OpmlCategory {
3535

3636
export async function readOpml(opmlContent: string): Promise<OpmlCategory[]> {
3737
const input = await parseXML(opmlContent)
38+
if (!input.opml || !input.opml.body || !input.opml.body[0] || !input.opml.body[0].outline) {
39+
throw new Error('Invalid OPML format: missing required structure')
40+
}
41+
3842
const body = input.opml.body
3943
const outlines = body[0].outline
4044

4145
const rootSubscriptions = outlines
42-
.filter((item: any) => item.$.type === 'rss')
46+
.filter((item: any) => item.$ && item.$.type === 'rss')
4347
.map((item: any) => item.$)
4448
const categories = outlines
45-
.filter((item: any) => item.$.type !== 'rss')
49+
.filter((item: any) => item.$ && item.$.type !== 'rss')
4650
.reduce((out: OpmlCategory[], outline: any) => {
51+
if (!outline.$ || !outline.$.title) {
52+
return out
53+
}
4754
const category = outline.$.title
4855
const items = outline.outline
4956
out.push({
@@ -52,7 +59,7 @@ export async function readOpml(opmlContent: string): Promise<OpmlCategory[]> {
5259
items &&
5360
items
5461
.map((item: any) => item.$)
55-
.filter((item: any) => item.type === 'rss')
62+
.filter((item: any) => item && item.type === 'rss')
5663
})
5764
return out
5865
}, [])

‎action/feeds/parsers#parseAtom-edge-cases.test.ts‎

Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,140 @@
1+
import test from 'ava'
2+
import { parseAtom, parseXML } from './parsers'
3+
4+
test('#parseAtom handles missing entry field gracefully', async (t) => {
5+
const xml = {
6+
feed: {
7+
title: ['Test Feed'],
8+
subtitle: ['Test subtitle'],
9+
link: [{ $: { rel: 'alternate', href: 'https://example.com' } }],
10+
updated: ['2024-01-01T00:00:00Z'],
11+
generator: ['test']
12+
// Note: entry field is missing
13+
}
14+
}
15+
16+
const site = parseAtom('Test Feed', xml)
17+
t.truthy(site)
18+
t.is(site.entries.length, 0)
19+
})
20+
21+
test('#parseAtom handles empty date fields gracefully', async (t) => {
22+
const xml = {
23+
feed: {
24+
title: ['Test Feed'],
25+
subtitle: ['Test subtitle'],
26+
link: [{ $: { rel: 'alternate', href: 'https://example.com' } }],
27+
updated: [''],
28+
generator: ['test'],
29+
entry: [
30+
{
31+
title: ['Test Entry'],
32+
link: [{ $: { rel: 'alternate', href: 'https://example.com/entry' } }],
33+
published: [''],
34+
updated: [''],
35+
content: [{ _: 'Test content' }]
36+
}
37+
]
38+
}
39+
}
40+
41+
const site = parseAtom('Test Feed', xml)
42+
t.truthy(site)
43+
t.false(isNaN(site.updatedAt))
44+
t.is(site.entries.length, 1)
45+
t.false(isNaN(site.entries[0].date))
46+
})
47+
48+
test('#parseAtom handles invalid date fields gracefully', async (t) => {
49+
const xml = {
50+
feed: {
51+
title: ['Test Feed'],
52+
subtitle: ['Test subtitle'],
53+
link: [{ $: { rel: 'alternate', href: 'https://example.com' } }],
54+
updated: ['not a valid date'],
55+
generator: ['test'],
56+
entry: [
57+
{
58+
title: ['Test Entry'],
59+
link: [{ $: { rel: 'alternate', href: 'https://example.com/entry' } }],
60+
published: ['invalid'],
61+
content: [{ _: 'Test content' }]
62+
}
63+
]
64+
}
65+
}
66+
67+
const site = parseAtom('Test Feed', xml)
68+
t.truthy(site)
69+
t.false(isNaN(site.updatedAt))
70+
t.is(site.entries.length, 1)
71+
t.false(isNaN(site.entries[0].date))
72+
})
73+
74+
test('#parseAtom handles empty entry array', async (t) => {
75+
const xml = {
76+
feed: {
77+
title: ['Test Feed'],
78+
subtitle: ['Test subtitle'],
79+
link: [{ $: { rel: 'alternate', href: 'https://example.com' } }],
80+
updated: ['2024-01-01T00:00:00Z'],
81+
generator: ['test'],
82+
entry: []
83+
}
84+
}
85+
86+
const site = parseAtom('Test Feed', xml)
87+
t.truthy(site)
88+
t.is(site.entries.length, 0)
89+
})
90+
91+
test('#parseAtom handles missing link in entry', async (t) => {
92+
const xml = {
93+
feed: {
94+
title: ['Test Feed'],
95+
subtitle: ['Test subtitle'],
96+
link: [{ $: { rel: 'alternate', href: 'https://example.com' } }],
97+
updated: ['2024-01-01T00:00:00Z'],
98+
generator: ['test'],
99+
entry: [
100+
{
101+
title: ['Test Entry'],
102+
// Note: link field is missing
103+
published: ['2024-01-01T00:00:00Z'],
104+
content: [{ _: 'Test content' }]
105+
}
106+
]
107+
}
108+
}
109+
110+
const site = parseAtom('Test Feed', xml)
111+
t.truthy(site)
112+
t.is(site.entries.length, 1)
113+
t.is(site.entries[0].link, '')
114+
})
115+
116+
test('#parseAtom handles empty link array in entry', async (t) => {
117+
const xml = {
118+
feed: {
119+
title: ['Test Feed'],
120+
subtitle: ['Test subtitle'],
121+
link: [{ $: { rel: 'alternate', href: 'https://example.com' } }],
122+
updated: ['2024-01-01T00:00:00Z'],
123+
generator: ['test'],
124+
entry: [
125+
{
126+
title: ['Test Entry'],
127+
link: [], // Empty link array
128+
published: ['2024-01-01T00:00:00Z'],
129+
content: [{ _: 'Test content' }]
130+
}
131+
]
132+
}
133+
}
134+
135+
const site = parseAtom('Test Feed', xml)
136+
t.truthy(site)
137+
t.is(site.entries.length, 1)
138+
t.is(site.entries[0].link, '')
139+
})
140+

‎action/feeds/parsers#parseRss-date.test.ts‎

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
import test from 'ava'
2+
import { parseRss, parseXML } from './parsers'
3+
4+
test('#parseRss handles empty date fields gracefully', async (t) => {
5+
const xml = {
6+
rss: {
7+
channel: [
8+
{
9+
link: ['https://example.com'],
10+
description: ['Test feed'],
11+
lastBuildDate: [''],
12+
generator: ['test'],
13+
item: [
14+
{
15+
title: ['Test Entry'],
16+
link: ['https://example.com/entry'],
17+
pubDate: [''],
18+
description: ['Test content']
19+
}
20+
]
21+
}
22+
]
23+
}
24+
}
25+
26+
const site = parseRss('Test Feed', xml)
27+
t.truthy(site)
28+
t.false(isNaN(site.updatedAt))
29+
t.is(site.entries.length, 1)
30+
t.false(isNaN(site.entries[0].date))
31+
})
32+
33+
test('#parseRss handles invalid date fields gracefully', async (t) => {
34+
const xml = {
35+
rss: {
36+
channel: [
37+
{
38+
link: ['https://example.com'],
39+
description: ['Test feed'],
40+
lastBuildDate: ['invalid date'],
41+
generator: ['test'],
42+
item: [
43+
{
44+
title: ['Test Entry'],
45+
link: ['https://example.com/entry'],
46+
pubDate: ['not a real date'],
47+
description: ['Test content']
48+
}
49+
]
50+
}
51+
]
52+
}
53+
}
54+
55+
const site = parseRss('Test Feed', xml)
56+
t.truthy(site)
57+
t.false(isNaN(site.updatedAt))
58+
t.is(site.entries.length, 1)
59+
t.false(isNaN(site.entries[0].date))
60+
})
61+
62+
test('#parseRss handles missing date fields gracefully', async (t) => {
63+
const xml = {
64+
rss: {
65+
channel: [
66+
{
67+
link: ['https://example.com'],
68+
description: ['Test feed'],
69+
generator: ['test'],
70+
item: [
71+
{
72+
title: ['Test Entry'],
73+
link: ['https://example.com/entry'],
74+
description: ['Test content']
75+
}
76+
]
77+
}
78+
]
79+
}
80+
}
81+
82+
const site = parseRss('Test Feed', xml)
83+
t.truthy(site)
84+
t.false(isNaN(site.updatedAt))
85+
t.is(site.entries.length, 1)
86+
t.false(isNaN(site.entries[0].date))
87+
})

0 commit comments

Comments
 (0)