openvibers/openvibe.chat Commits
Signed-out reads keep only the per-address limit (900 a minute). On Live's busiest streams many viewers share one carrier or campus address, and a per-actor budget of 120 a minute for all of them would refuse real people. Signed-in reads, bots and every write keep their per-actor numbers. test/actor-limits.test.js: eight signed-out reads from one address all pass, and no chat.read refusal is counted. npm test passes.
Changes
@@@ -13,9 +13,9 @@* hooks at /internal/calls) carry every viewer's chat and are never limited here: a refusal there* would drop the whole site's chat, and the WebSocket keeps its own flood control.*-- * Reads: every GET/HEAD under /api/chat, /api/dm, /api/tts, /api/sounds (and /api/streams once calls-- * are Chat's) takes CHAT_LIMITS_MINUTE / CHAT_LIMITS_HOUR, 120 and 3000, one budget per API, counted-- * once the caller's token is resolved (identify below). Writes and expensive reads set their own,++ * Reads: every signed-in GET/HEAD under /api/chat, /api/dm, /api/tts, /api/sounds (and /api/streams once++ * calls are Chat's) takes CHAT_LIMITS_MINUTE / CHAT_LIMITS_HOUR, 120 and 3000, one budget per API, counted++ * once the caller's token is resolved (identify below). Signed-out reads keep the per-address limit only. Writes and expensive reads set their own,* tighter numbers where they are mounted, after requireAuth. Past a limit the route answers 429* problem+json `rate_limited` with Retry-After before it does any work; the refusal is logged once* and counted in chat_rate_limited_total{limit,window}. Counters live in this process: a restart@@@ -72,7 +72,10 @@ limits.identify = async function identify(req, res, next) {*/limits.reads = function reads(name, { skip = null } = {}) {const limit = limits(name);-- return (req, res, next) => ((req.method === 'GET' || req.method === 'HEAD') && !(skip && skip(req)) ? limit(req, res, next) : next());++ // Signed-out reads stay with the per-address limit (900 a minute, app.js): on Live's busiest streams many++ // viewers share one carrier or campus address, and 120 a minute for all of them would refuse real people.++ const signedOut = (req) => !personOf(req.user) && !personOf(req.ovActorUser);++ return (req, res, next) => ((req.method === 'GET' || req.method === 'HEAD') && !signedOut(req) && !(skip && skip(req)) ? limit(req, res, next) : next());};/** Count refusals in this app's registry (chat_rate_limited_total{limit,window}). */@@@ -3,7 +3,7 @@* Per-actor rate limits on the REST API (server/net/actor-limits.js, roadmap WS-R task 4): past its* limit one caller gets 429 problem+json `rate_limited` with Retry-After, before the route does any* work, while another caller still passes; each API has its own read budget and the window reopens-- * on the clock. A person's API token counts against the person; signed-out callers count by address;++ * on the clock. A person's API token counts against the person; signed-out reads are left to the per-address limit;* media files are left to the per-address limit. Writes have their own, tighter numbers. Health,* ready, release.json, metrics and /internal/* are never limited; refusals are logged and counted.*/@@@ -42,11 +42,9 @@ t('the person\'s API token counts against the person; another API has its own buassert.strictEqual((await h.http('GET', '/api/chat/online?users=alice', { token: alice.token })).status, 200, '/api/chat is another budget');});--t('signed-out callers count by address; media files are left to the per-address limit', async () => {++t('signed-out reads keep only the per-address limit (many viewers share an address); media files too', async () => {const from = (ip) => h.http('GET', '/api/chat/online?users=alice', { headers: { 'X-Forwarded-For': ip } });-- for (let i = 0; i < 3; i++) assert.strictEqual((await from('203.0.113.7')).status, 200);-- assert.strictEqual((await from('203.0.113.7')).status, 429);-- assert.strictEqual((await from('203.0.113.8')).status, 200, 'another address still passes');++ for (let i = 0; i < 8; i++) assert.strictEqual((await from('203.0.113.7')).status, 200, `signed-out read ${i + 1}`);for (let i = 0; i < 6; i++) {assert.strictEqual((await h.http('GET', '/api/sounds/file/none.mp3', { headers: { 'X-Forwarded-For': '203.0.113.9' } })).status, 404);}@@@ -88,7 +86,7 @@ t('refusals are counted in chat_rate_limited_total', async () => {const m = (await h.http('GET', '/metrics')).text;const lines = m.split('\n').filter((l) => l.includes('chat_rate_limited_total')).join('\n');assert.ok(/chat_rate_limited_total\{limit="chat.dm.read",window="minute"\} 2/.test(m), lines);-- assert.ok(/chat_rate_limited_total\{limit="chat.read",window="minute"\} 1/.test(m), lines);++ assert.ok(!/limit="chat.read"/.test(m), 'signed-out reads were never refused per actor');assert.ok(/chat_rate_limited_total\{limit="chat.message.send",window="minute"\} 1/.test(m), lines);});
@@ -13,9 +13,9 @@
@@ -13,9 +13,9 @@
* hooks at /internal/calls) carry every viewer's chat and are never limited here: a refusal there
* hooks at /internal/calls) carry every viewer's chat and are never limited here: a refusal there
* would drop the whole site's chat, and the WebSocket keeps its own flood control.
* would drop the whole site's chat, and the WebSocket keeps its own flood control.
*
*
- * Reads: every GET/HEAD under /api/chat, /api/dm, /api/tts, /api/sounds (and /api/streams once calls
- * are Chat's) takes CHAT_LIMITS_MINUTE / CHAT_LIMITS_HOUR, 120 and 3000, one budget per API, counted
- * once the caller's token is resolved (identify below). Writes and expensive reads set their own,
+ * Reads: every signed-in GET/HEAD under /api/chat, /api/dm, /api/tts, /api/sounds (and /api/streams once
+ * calls are Chat's) takes CHAT_LIMITS_MINUTE / CHAT_LIMITS_HOUR, 120 and 3000, one budget per API, counted
+ * once the caller's token is resolved (identify below). Signed-out reads keep the per-address limit only. Writes and expensive reads set their own,
* tighter numbers where they are mounted, after requireAuth. Past a limit the route answers 429
* tighter numbers where they are mounted, after requireAuth. Past a limit the route answers 429
* problem+json `rate_limited` with Retry-After before it does any work; the refusal is logged once
* problem+json `rate_limited` with Retry-After before it does any work; the refusal is logged once
* and counted in chat_rate_limited_total{limit,window}. Counters live in this process: a restart
* and counted in chat_rate_limited_total{limit,window}. Counters live in this process: a restart
@@ -72,7 +72,10 @@ limits.identify = async function identify(req, res, next) {
@@ -72,7 +72,10 @@ limits.identify = async function identify(req, res, next) {
*/
*/
limits.reads = function reads(name, { skip = null } = {}) {
limits.reads = function reads(name, { skip = null } = {}) {
const limit = limits(name);
const limit = limits(name);
- return (req, res, next) => ((req.method === 'GET' || req.method === 'HEAD') && !(skip && skip(req)) ? limit(req, res, next) : next());
+ // Signed-out reads stay with the per-address limit (900 a minute, app.js): on Live's busiest streams many
+ // viewers share one carrier or campus address, and 120 a minute for all of them would refuse real people.
+ const signedOut = (req) => !personOf(req.user) && !personOf(req.ovActorUser);
+ return (req, res, next) => ((req.method === 'GET' || req.method === 'HEAD') && !signedOut(req) && !(skip && skip(req)) ? limit(req, res, next) : next());
};
};
/** Count refusals in this app's registry (chat_rate_limited_total{limit,window}). */
/** Count refusals in this app's registry (chat_rate_limited_total{limit,window}). */
@@ -3,7 +3,7 @@
@@ -3,7 +3,7 @@
* Per-actor rate limits on the REST API (server/net/actor-limits.js, roadmap WS-R task 4): past its
* Per-actor rate limits on the REST API (server/net/actor-limits.js, roadmap WS-R task 4): past its
* limit one caller gets 429 problem+json `rate_limited` with Retry-After, before the route does any
* limit one caller gets 429 problem+json `rate_limited` with Retry-After, before the route does any
* work, while another caller still passes; each API has its own read budget and the window reopens
* work, while another caller still passes; each API has its own read budget and the window reopens
- * on the clock. A person's API token counts against the person; signed-out callers count by address;
+ * on the clock. A person's API token counts against the person; signed-out reads are left to the per-address limit;
* media files are left to the per-address limit. Writes have their own, tighter numbers. Health,
* media files are left to the per-address limit. Writes have their own, tighter numbers. Health,
* ready, release.json, metrics and /internal/* are never limited; refusals are logged and counted.
* ready, release.json, metrics and /internal/* are never limited; refusals are logged and counted.
*/
*/
@@ -42,11 +42,9 @@ t('the person\'s API token counts against the person; another API has its own bu
@@ -42,11 +42,9 @@ t('the person\'s API token counts against the person; another API has its own bu
assert.strictEqual((await h.http('GET', '/api/chat/online?users=alice', { token: alice.token })).status, 200, '/api/chat is another budget');
assert.strictEqual((await h.http('GET', '/api/chat/online?users=alice', { token: alice.token })).status, 200, '/api/chat is another budget');
});
});
-t('signed-out callers count by address; media files are left to the per-address limit', async () => {
+t('signed-out reads keep only the per-address limit (many viewers share an address); media files too', async () => {
const from = (ip) => h.http('GET', '/api/chat/online?users=alice', { headers: { 'X-Forwarded-For': ip } });
const from = (ip) => h.http('GET', '/api/chat/online?users=alice', { headers: { 'X-Forwarded-For': ip } });
- for (let i = 0; i < 3; i++) assert.strictEqual((await from('203.0.113.7')).status, 200);
- assert.strictEqual((await from('203.0.113.7')).status, 429);
- assert.strictEqual((await from('203.0.113.8')).status, 200, 'another address still passes');
+ for (let i = 0; i < 8; i++) assert.strictEqual((await from('203.0.113.7')).status, 200, `signed-out read ${i + 1}`);
for (let i = 0; i < 6; i++) {
for (let i = 0; i < 6; i++) {
assert.strictEqual((await h.http('GET', '/api/sounds/file/none.mp3', { headers: { 'X-Forwarded-For': '203.0.113.9' } })).status, 404);
assert.strictEqual((await h.http('GET', '/api/sounds/file/none.mp3', { headers: { 'X-Forwarded-For': '203.0.113.9' } })).status, 404);
}
}
@@ -88,7 +86,7 @@ t('refusals are counted in chat_rate_limited_total', async () => {
@@ -88,7 +86,7 @@ t('refusals are counted in chat_rate_limited_total', async () => {
const m = (await h.http('GET', '/metrics')).text;
const m = (await h.http('GET', '/metrics')).text;
const lines = m.split('\n').filter((l) => l.includes('chat_rate_limited_total')).join('\n');
const lines = m.split('\n').filter((l) => l.includes('chat_rate_limited_total')).join('\n');
assert.ok(/chat_rate_limited_total\{limit="chat.dm.read",window="minute"\} 2/.test(m), lines);
assert.ok(/chat_rate_limited_total\{limit="chat.dm.read",window="minute"\} 2/.test(m), lines);
- assert.ok(/chat_rate_limited_total\{limit="chat.read",window="minute"\} 1/.test(m), lines);
+ assert.ok(!/limit="chat.read"/.test(m), 'signed-out reads were never refused per actor');
assert.ok(/chat_rate_limited_total\{limit="chat.message.send",window="minute"\} 1/.test(m), lines);
assert.ok(/chat_rate_limited_total\{limit="chat.message.send",window="minute"\} 1/.test(m), lines);
});
});