Add a maximum feed size

.eslintrc.js

@@ -6,6 +6,7 @@ module.exports = {
  ],
  extends: [
  'eslint:recommended',
+ 'plugin:mocha/recommended',
  'plugin:@typescript-eslint/recommended',
  ],
  // eslint-config-preact needs a Jest version to be happy, even if Jest isn't used.
@@ -18,7 +19,10 @@ module.exports = {
  "@typescript-eslint/no-explicit-any": "warn",
  "@typescript-eslint/no-unused-vars": "warn",
  "camelcase": ["error", { "properties": "never", "ignoreDestructuring": true }],
- "no-console": "error"
+ "no-console": "error",
+ "mocha/no-exclusive-tests": "error",
+ // We do this everywhere.
+ "mocha/no-mocha-arrows": "off",
  },
  env: {
  node: true,

changelog.d/906.feature

@@ -0,0 +1 @@
+Add a maximum allowed payload size to RSS feeds. This prevents oversized feeds from potentially slowing down Hookshot.

config.sample.yml

@@ -112,6 +112,9 @@ listeners:
 # pollConcurrency: 4
 # pollIntervalSeconds: 600
 # pollTimeoutSeconds: 30
+# maximumFeedSizeMB:
+# # (Optional) The maximum response size of a feed on first load. Oversized responses will prevent a connection from being created.
+# 25
 
 #provisioning:
 # # (Optional) Provisioning API for integration managers

package.json

@@ -102,7 +102,7 @@
  "chai": "^4.3.4",
  "eslint": "^8.49.0",
  "eslint-config-preact": "^1.3.0",
- "eslint-plugin-mocha": "^10.1.0",
+ "eslint-plugin-mocha": "^10.4.2",
  "homerunner-client": "^1.0.0",
  "jest": "^29.7.0",
  "mini.css": "^3.0.1",

src/Connections/FeedConnection.ts

@@ -10,6 +10,7 @@
 import { readFeed, sanitizeHtml } from "../libRs";
 import UserAgent from "../UserAgent";
 import { retry, retryMatrixErrorFilter } from "../PromiseUtil";
+import { BridgeConfigFeeds } from "../config/Config";
 const log = new Logger("FeedConnection");
 const md = new markdown({
  html: true,
@@ -57,14 +58,14 @@
  static readonly ServiceCategory = "feeds";
 
 
  public static createConnectionForState(roomId: string, event: StateEvent<any>, {config, intent}: InstantiateConnectionOpts) {
  if (!config.feeds?.enabled) {
  throw Error('RSS/Atom feeds are not configured');
  }
  return new FeedConnection(roomId, event.stateKey, event.content, intent);
  }
 
- static async validateUrl(url: string): Promise<void> {
+ static async validateUrl(url: string, config: BridgeConfigFeeds): Promise<void> {
  try {
  new URL(url);
  } catch (ex) {
@@ -75,6 +76,7 @@
  await readFeed(url, {
  userAgent: UserAgent,
  pollTimeoutSeconds: VALIDATION_FETCH_TIMEOUT_S,
+ maximumFeedSizeMb: config.maximumFeedSizeMB,
  });
  } catch (ex) {
  throw new ApiError(`Could not read feed from URL: ${ex.message}`, ErrCode.BadValue);
@@ -113,7 +115,7 @@
  }
 
  const state = this.validateState(data);
- await FeedConnection.validateUrl(state.url);
+ await FeedConnection.validateUrl(state.url, config.feeds);
  const connection = new FeedConnection(roomId, state.url, state, intent);
  await intent.underlyingClient.sendStateEvent(roomId, FeedConnection.CanonicalEventType, state.url, state);
 

src/Connections/SetupConnection.ts

@@ -312,7 +312,7 @@ export class SetupConnection extends CommandConnection {
 
  // provisionConnection will check it again, but won't give us a nice CommandError on failure
  try {
- await FeedConnection.validateUrl(url);
+ await FeedConnection.validateUrl(url, this.config.feeds);
  } catch (err: unknown) {
  log.debug(`Feed URL '${url}' failed validation: ${err}`);
  if (err instanceof ApiError) {

src/config/Config.ts

@@ -251,6 +251,7 @@ export interface BridgeConfigFeedsYAML {
  pollIntervalSeconds?: number;
  pollConcurrency?: number;
  pollTimeoutSeconds?: number;
+ maximumFeedSizeMB?: number;
 }
 
 export class BridgeConfigFeeds {
@@ -259,13 +260,24 @@ export class BridgeConfigFeeds {
  public pollTimeoutSeconds: number;
  public pollConcurrency: number;
 
+ @configKey("The maximum response size of a feed on first load. Oversized responses will prevent a connection from being created.", true)
+ public maximumFeedSizeMB: number;
+
  constructor(yaml: BridgeConfigFeedsYAML) {
  this.enabled = yaml.enabled;
  this.pollConcurrency = yaml.pollConcurrency ?? 4;
  this.pollIntervalSeconds = yaml.pollIntervalSeconds ?? 600;
  assert.strictEqual(typeof this.pollIntervalSeconds, "number");
  this.pollTimeoutSeconds = yaml.pollTimeoutSeconds ?? 30;
  assert.strictEqual(typeof this.pollTimeoutSeconds, "number");
+ this.maximumFeedSizeMB = yaml.maximumFeedSizeMB ?? 25;
+ assert.strictEqual(typeof this.maximumFeedSizeMB, "number");
+ if (this.maximumFeedSizeMB < 1) {
+ throw new ConfigError('feeds.maximumFeedSizeMB', 'Must be at least 1MB or greater');
+ }
+ if (!Number.isInteger(this.maximumFeedSizeMB)) {
+ throw new ConfigError('feeds.maximumFeedSizeMB', 'Must be a whole number');
+ }
  }
 
  @hideKey()

src/config/Defaults.ts

@@ -124,6 +124,7 @@ export const DefaultConfigRoot: BridgeConfigRoot = {
  pollIntervalSeconds: 600,
  pollTimeoutSeconds: 30,
  pollConcurrency: 4,
+ maximumFeedSizeMB: 25,
  },
  provisioning: {
  secret: "!secretToken"

src/feeds/FeedReader.ts

@@ -215,6 +215,7 @@ export class FeedReader {
  etag,
  lastModified,
  userAgent: UserAgent,
+ maximumFeedSizeMb: this.config.maximumFeedSizeMB,
  });
 
  // Store any entity tags/cache times.

src/feeds/parser.rs

@@ -37,6 +37,7 @@ pub struct ReadFeedOptions {
  pub etag: Option<String>,
  pub poll_timeout_seconds: i64,
  pub user_agent: String,
+ pub maximum_feed_size_mb: i64,
 }
 
 #[derive(Serialize, Debug, Deserialize)]
@@ -199,25 +200,41 @@ pub async fn js_read_feed(url: String, options: ReadFeedOptions) -> Result<FeedR
  headers.append("If-None-Match", HeaderValue::from_str(&etag).unwrap());
  }
 
+ let max_content_size = (options.maximum_feed_size_mb * 1024 * 1024) as u64;
+
  match req.headers(headers).send().await {
  Ok(res) => {
+ // Pre-emptive check
+ let content_length = res.content_length().unwrap_or(0);
+ if content_length > max_content_size {
+ return Err(JsError::new(Status::Unknown, "Feed exceeded maximum size"));
+ }
+
  let res_headers = res.headers().clone();
  match res.status() {
  StatusCode::OK => match res.text().await {
- Ok(body) => match js_parse_feed(body) {
- Ok(feed) => Ok(FeedResult {
- feed: Some(feed),
- etag: res_headers
- .get("ETag")
- .map(|v| v.to_str().unwrap())
- .map(|v| v.to_string()),
- last_modified: res_headers
- .get("Last-Modified")
- .map(|v| v.to_str().unwrap())
- .map(|v| v.to_string()),
- }),
- Err(err) => Err(err),
- },
+ Ok(body) => {
+ // Check if we only got the length after loading the response.
+ match body.len() as u64 <= max_content_size {
+ true => match js_parse_feed(body) {
+ Ok(feed) => Ok(FeedResult {
+ feed: Some(feed),
+ etag: res_headers
+ .get("ETag")
+ .map(|v| v.to_str().unwrap())
+ .map(|v| v.to_string()),
+ last_modified: res_headers
+ .get("Last-Modified")
+ .map(|v| v.to_str().unwrap())
+ .map(|v| v.to_string()),
+ }),
+ Err(err) => Err(err),
+ },
+ false => {
+ Err(JsError::new(Status::Unknown, "Feed exceeded maximum size"))
+ }
+ }
+ }
  Err(err) => Err(JsError::new(Status::Unknown, err)),
  },
  StatusCode::NOT_MODIFIED => Ok(FeedResult {

tests/FeedReader.spec.ts

@@ -1,6 +1,6 @@
 import { expect } from "chai";
 import EventEmitter from "events";
-import { BridgeConfigFeeds } from "../src/config/Config";
+import { BridgeConfigFeeds, BridgeConfigFeedsYAML } from "../src/config/Config";
 import { ConnectionManager } from "../src/ConnectionManager";
 import { IConnection } from "../src/Connections";
 import { FeedEntry, FeedReader } from "../src/feeds/FeedReader";
@@ -39,14 +39,15 @@ class MockMessageQueue extends EventEmitter implements MessageQueue {
  }
 }
 
-async function constructFeedReader(feedResponse: () => {headers: Record<string,string>, data: string}) {
+async function constructFeedReader(feedResponse: () => {headers?: Record<string,string>, data: string}, extraConfig?: Partial<BridgeConfigFeedsYAML>) {
+
  const httpServer = await new Promise<Server>(resolve => {
  const srv = createServer((_req, res) => {
- res.writeHead(200);
  const { headers, data } = feedResponse();
- Object.entries(headers).forEach(([key,value]) => {
+ Object.entries(headers ?? {}).forEach(([key,value]) => {
  res.setHeader(key, value);
  });
+ res.writeHead(200);
  res.write(data);
  res.end();
  }).listen(0, '127.0.0.1', () => {
@@ -59,6 +60,7 @@ async function constructFeedReader(feedResponse: () => {headers: Record<string,s
  enabled: true,
  pollIntervalSeconds: 1,
  pollTimeoutSeconds: 1,
+ ...extraConfig,
  });
  const cm = new MockConnectionManager([{ feedUrl } as unknown as IConnection]) as unknown as ConnectionManager
  const mq = new MockMessageQueue();
@@ -71,7 +73,12 @@ async function constructFeedReader(feedResponse: () => {headers: Record<string,s
  const feedReader = new FeedReader(
  config, cm, mq, storage,
  );
- after(() => httpServer.close());
+
+ after(() => {
+ httpServer.close()
+ feedReader.stop();
+ });
+
  return {config, cm, events, feedReader, feedUrl, httpServer, storage}; 
 }
 
@@ -94,12 +101,12 @@ describe("FeedReader", () => {
  }));
 
  await feedReader.pollFeed(feedUrl);
- feedReader.stop();
  expect(events).to.have.lengthOf(1);
 
  expect(events[0].data.feed.title).to.equal(null);
  expect(events[0].data.title).to.equal(null);
  });
+
  it("should handle RSS 2.0 feeds", async () => {
  const { events, feedReader, feedUrl } = await constructFeedReader(() => ({
  headers: {}, data: `
@@ -127,7 +134,6 @@ describe("FeedReader", () => {
  }));
 
  await feedReader.pollFeed(feedUrl);
- feedReader.stop();
  expect(events).to.have.lengthOf(1);
 
  expect(events[0].data.feed.title).to.equal('RSS Title');
@@ -137,6 +143,7 @@ describe("FeedReader", () => {
  expect(events[0].data.link).to.equal('http://www.example.com/blog/post/1');
  expect(events[0].data.pubdate).to.equal('Sun, 6 Sep 2009 16:20:00 +0000');
  });
+
  it("should handle RSS feeds with a permalink url", async () => {
  const { events, feedReader, feedUrl } = await constructFeedReader(() => ({
  headers: {}, data: `
@@ -163,7 +170,6 @@ describe("FeedReader", () => {
  }));
 
  await feedReader.pollFeed(feedUrl);
- feedReader.stop();
  expect(events).to.have.lengthOf(1);
 
  expect(events[0].data.feed.title).to.equal('RSS Title');
@@ -173,6 +179,7 @@ describe("FeedReader", () => {
  expect(events[0].data.link).to.equal('http://www.example.com/blog/post/1');
  expect(events[0].data.pubdate).to.equal('Sun, 6 Sep 2009 16:20:00 +0000');
  });
+
  it("should handle Atom feeds", async () => {
  const { events, feedReader, feedUrl } = await constructFeedReader(() => ({
  headers: {}, data: `
@@ -203,7 +210,6 @@ describe("FeedReader", () => {
  }));
 
  await feedReader.pollFeed(feedUrl);
- feedReader.stop();
  expect(events).to.have.lengthOf(1);
 
  expect(events[0].data.feed.title).to.equal('Example Feed');
@@ -213,6 +219,7 @@ describe("FeedReader", () => {
  expect(events[0].data.link).to.equal('http://example.org/2003/12/13/atom03');
  expect(events[0].data.pubdate).to.equal('Sat, 13 Dec 2003 18:30:02 +0000');
  });
+
  it("should not duplicate feed entries", async () => {
  const { events, feedReader, feedUrl } = await constructFeedReader(() => ({
  headers: {}, data: `
@@ -235,9 +242,9 @@ describe("FeedReader", () => {
  await feedReader.pollFeed(feedUrl);
  await feedReader.pollFeed(feedUrl);
  await feedReader.pollFeed(feedUrl);
- feedReader.stop();
  expect(events).to.have.lengthOf(1);
  });
+
  it("should always hash to the same value for Atom feeds", async () => {
  const expectedHash = ['md5:d41d8cd98f00b204e9800998ecf8427e'];
  const { feedReader, feedUrl, storage } = await constructFeedReader(() => ({
@@ -254,10 +261,10 @@ describe("FeedReader", () => {
  }));
 
  await feedReader.pollFeed(feedUrl);
- feedReader.stop();
  const items = await storage.hasSeenFeedGuids(feedUrl, ...expectedHash);
  expect(items).to.deep.equal(expectedHash);
  });
+
  it("should always hash to the same value for RSS feeds", async () => {
  const expectedHash = [
  'md5:98bafde155b931e656ad7c137cd7711e', // guid
@@ -288,8 +295,53 @@ describe("FeedReader", () => {
  }));
 
  await feedReader.pollFeed(feedUrl);
- feedReader.stop();
  const items = await storage.hasSeenFeedGuids(feedUrl, ...expectedHash);
  expect(items).to.deep.equal(expectedHash);
  });
+
+ it("should fail to handle a feed which exceed the maximum size.", async () => {
+ // Create some data of the right length
+ const data = `
+ <?xml version="1.0" encoding="UTF-8" ?>
+ <rss version="2.0">
+ <channel>
+ <title>RSS Title</title>
+ <description>This is an example of an RSS feed</description>
+ ${Array.from({length: 8000}).map((_, i) => `<item>
+ <title>Example entry</title>
+ <guid isPermaLink="true">http://www.example.com/blog/post/${i}</guid>
+ </item>`).join('')}
+ </channel>
+ </rss>`;
+ const { feedReader, feedUrl } = await constructFeedReader(() => ({
+ data, headers: { 'Content-Length': data.length.toString()}
+ }), {
+ maximumFeedSizeMB: 1
+ });
+ await feedReader.pollFeed(feedUrl);
+ expect(feedReader["feedsFailingParsing"]).to.contain(feedUrl);
+ });
+
+ it("should fail to handle a feed which exceed the maximum size which does NOT send a Content-Length.", async () => {
+ // Create some data of the right length
+ const data = `
+ <?xml version="1.0" encoding="UTF-8" ?>
+ <rss version="2.0">
+ <channel>
+ <title>RSS Title</title>
+ <description>This is an example of an RSS feed</description>
+ ${Array.from({length: 8000}).map((_, i) => `<item>
+ <title>Example entry</title>
+ <guid isPermaLink="true">http://www.example.com/blog/post/${i}</guid>
+ </item>`).join('')}
+ </channel>
+ </rss>`;
+ const { feedReader, feedUrl } = await constructFeedReader(() => ({
+ data
+ }), {
+ maximumFeedSizeMB: 1
+ });
+ await feedReader.pollFeed(feedUrl);
+ expect(feedReader["feedsFailingParsing"]).to.contain(feedUrl);
+ });
 });