Skip to content

Commit

Permalink
Convert snapshot file name serialization to hashes (#3279)
Browse files Browse the repository at this point in the history
Snapshot file names were originally encoded in base64. This caused a
problem for longer request URIs because the file name would grow with
the size of the request.

by switching to sha256 message digests for the file names, we avoid this
issue of file name size growth.

This problem affects file systems that have a smaller maximum file name
lengths. The max allowable is 255 chars. The user reporting an issue had
theirs set to 143. sha256 message digests are all 64 chars in length.

For good measure, all snapshot files with hashed names are now excluded
from the npm package so only developers cloning our repo might need to
worry about any of this going forward.

test plan:

unit tests should all still pass.
  • Loading branch information
topocount committed Dec 6, 2021
1 parent 4effd92 commit 47af83f
Show file tree
Hide file tree
Showing 40 changed files with 1,177 additions and 609 deletions.
3 changes: 2 additions & 1 deletion packages/sourcecred/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,8 @@
"/dist",
"/src",
"!src/**/*.test.js?(.snap)",
"!src/ui"
"!src/ui",
"!src/**/snapshots"
],
"main": "dist/server/api.js",
"browser": "dist/client/api.js",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ Array [
Object {
"nick": null,
"roles": Array [
"678350026946117694",
"678349848684003359",
],
"user": Object {
Expand Down Expand Up @@ -267,6 +268,18 @@ Array [
"username": "sourcecred-test-thena",
},
},
Object {
"nick": null,
"roles": Array [
"678349848684003359",
],
"user": Object {
"bot": false,
"discriminator": "4121",
"id": "773595284894253136",
"username": "amrro",
},
},
]
`;

Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
// @flow

import base64url from "base64url";
import path from "path";
import fs from "fs-extra";
import {Fetcher} from "./fetcher";
import {createHash} from "crypto";

async function snapshotFetch(url: string | Request | URL): Promise<Response> {
const snapshotDir = "src/plugins/discord/snapshots";
const filename = base64url(url);
const shasum = createHash("sha256");
shasum.update(Buffer.from(((url: any): string), "utf8"));
const filename = shasum.digest("hex");
const file = path.join(snapshotDir, filename);
if (await fs.exists(file)) {
const contents = await fs.readFile(file);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
"joined_at": "2020-02-15T21:16:40.215000+00:00",
"is_pending": false,
"pending": false,
"communication_disabled_until": null,
"user": {
"id": "143776454050709505",
"username": "Beanow",
Expand All @@ -22,6 +23,7 @@
},
{
"roles": [
"678350026946117694",
"678349848684003359"
],
"nick": null,
Expand All @@ -30,6 +32,7 @@
"joined_at": "2020-11-02T18:02:40.116000+00:00",
"is_pending": false,
"pending": false,
"communication_disabled_until": null,
"user": {
"id": "258786206387666944",
"username": "Thena",
Expand All @@ -50,10 +53,11 @@
"joined_at": "2021-04-27T01:08:45.585000+00:00",
"is_pending": false,
"pending": false,
"communication_disabled_until": null,
"user": {
"id": "318552163649454081",
"username": "hz",
"avatar": "4dd1aa262e7e332bf2680428f03793bf",
"avatar": "a7495c44681943eaea928c5e0194b1cb",
"discriminator": "8826",
"public_flags": 0
},
Expand All @@ -71,10 +75,11 @@
"joined_at": "2020-02-15T22:31:31.243000+00:00",
"is_pending": false,
"pending": false,
"communication_disabled_until": null,
"user": {
"id": "420341518948237331",
"username": "dandelion",
"avatar": "43653169c81b2fdbb9b93cc39d1f0e2a",
"avatar": "03a1489e7895e88e0555a7c19c3ecee7",
"discriminator": "3370",
"public_flags": 0
},
Expand All @@ -92,6 +97,7 @@
"joined_at": "2020-02-15T22:21:55.613000+00:00",
"is_pending": false,
"pending": false,
"communication_disabled_until": null,
"user": {
"id": "432981598858903585",
"username": "wchargin",
Expand All @@ -113,6 +119,7 @@
"joined_at": "2020-02-16T00:45:23.478000+00:00",
"is_pending": false,
"pending": false,
"communication_disabled_until": null,
"user": {
"id": "439050857921904640",
"username": "Brian Litwin",
Expand All @@ -134,6 +141,7 @@
"joined_at": "2020-10-02T18:07:37.732000+00:00",
"is_pending": false,
"pending": false,
"communication_disabled_until": null,
"user": {
"id": "489154203714060300",
"username": "topocount",
Expand All @@ -154,6 +162,7 @@
"joined_at": "2020-10-02T18:07:44.893000+00:00",
"is_pending": false,
"pending": false,
"communication_disabled_until": null,
"user": {
"id": "579830927287386126",
"username": "sandpiper",
Expand All @@ -172,6 +181,7 @@
"joined_at": "2021-05-10T10:30:53.883000+00:00",
"is_pending": false,
"pending": false,
"communication_disabled_until": null,
"user": {
"id": "664562775770791966",
"username": "BrianBelhumeur",
Expand All @@ -192,6 +202,7 @@
"joined_at": "2020-02-15T21:57:23.609000+00:00",
"is_pending": false,
"pending": false,
"communication_disabled_until": null,
"user": {
"id": "678351352770068560",
"username": "CredBot-Beanow",
Expand All @@ -213,6 +224,7 @@
"joined_at": "2020-07-03T22:33:33.496000+00:00",
"is_pending": false,
"pending": false,
"communication_disabled_until": null,
"user": {
"id": "728738433199112280",
"username": "sts-credbot",
Expand All @@ -234,6 +246,7 @@
"joined_at": "2020-10-20T00:39:44.545000+00:00",
"is_pending": false,
"pending": false,
"communication_disabled_until": null,
"user": {
"id": "758370311774404618",
"username": "topocount's server bot",
Expand All @@ -255,6 +268,7 @@
"joined_at": "2020-10-02T21:48:29.344000+00:00",
"is_pending": false,
"pending": false,
"communication_disabled_until": null,
"user": {
"id": "761705425870782516",
"username": "SourceCred Test",
Expand All @@ -276,6 +290,7 @@
"joined_at": "2020-11-02T18:31:13.848000+00:00",
"is_pending": false,
"pending": false,
"communication_disabled_until": null,
"user": {
"id": "772886503830323210",
"username": "sourcecred-test-thena",
Expand All @@ -286,5 +301,26 @@
},
"mute": false,
"deaf": false
},
{
"roles": [
"678349848684003359"
],
"nick": null,
"avatar": null,
"premium_since": null,
"joined_at": "2021-12-02T16:16:58.266000+00:00",
"is_pending": false,
"pending": false,
"communication_disabled_until": null,
"user": {
"id": "773595284894253136",
"username": "amrro",
"avatar": "8a698ef8a269f95d184b7cc4d50368bf",
"discriminator": "4121",
"public_flags": 0
},
"mute": false,
"deaf": false
}
]
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,7 @@
"rules_channel_id": "830088231538130964",
"public_updates_channel_id": "830088231538130965",
"hub_type": null,
"premium_progress_bar_enabled": false,
"nsfw": false,
"nsfw_level": 0,
"embed_enabled": false,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@
"owner": false,
"permissions": 33620992,
"features": [
"NEWS",
"COMMUNITY"
"COMMUNITY",
"NEWS"
],
"permissions_new": "1071627961344"
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
},
{
"id": "678348980849213472",
"last_message_id": "841261006906327070",
"last_message_id": "916000597478629466",
"type": 0,
"name": "general",
"position": 0,
Expand All @@ -30,7 +30,8 @@
"guild_id": "678348980639498428",
"permission_overwrites": [],
"nsfw": false,
"rate_limit_per_user": 0
"rate_limit_per_user": 0,
"banner": null
},
{
"id": "678348980878573661",
Expand All @@ -57,7 +58,8 @@
"guild_id": "678348980639498428",
"permission_overwrites": [],
"nsfw": false,
"rate_limit_per_user": 0
"rate_limit_per_user": 0,
"banner": null
},
{
"id": "678696874869522446",
Expand All @@ -70,7 +72,8 @@
"guild_id": "678348980639498428",
"permission_overwrites": [],
"nsfw": false,
"rate_limit_per_user": 0
"rate_limit_per_user": 0,
"banner": null
},
{
"id": "830088231538130964",
Expand All @@ -92,11 +95,12 @@
}
],
"nsfw": false,
"rate_limit_per_user": 0
"rate_limit_per_user": 0,
"banner": null
},
{
"id": "830088231538130965",
"last_message_id": "870202554259214396",
"last_message_id": "909355881186066502",
"type": 0,
"name": "moderator-only",
"position": 0,
Expand All @@ -114,7 +118,8 @@
}
],
"nsfw": false,
"rate_limit_per_user": 0
"rate_limit_per_user": 0,
"banner": null
},
{
"id": "830089044695187586",
Expand Down Expand Up @@ -150,6 +155,7 @@
"guild_id": "678348980639498428",
"permission_overwrites": [],
"nsfw": true,
"rate_limit_per_user": 0
"rate_limit_per_user": 0,
"banner": null
}
]
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ cd "${CORE_PATH}"

fetch() {
url="${test_instance_url}$1"
filename="$(printf '%s' "${url}" | base64 -w 0 | tr -d '=' | tr '/+' '_-')"
filename="$(printf '%s' "${url}" | shasum -a 256 -U | cut -d' ' -f1)"
path="${snapshots_dir}/${filename}"
curl -sfL "$url" \
-H "Accept: application/json" \
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// @flow

import deepFreeze from "deep-freeze";
import base64url from "base64url";
import {createHash} from "crypto";
import path from "path";
import fs from "fs-extra";
import {Fetcher, type DiscourseFetchOptions} from "./fetch";
Expand All @@ -12,7 +12,9 @@ export const options: DiscourseFetchOptions = deepFreeze({

async function snapshotFetch(url: string | Request | URL): Promise<Response> {
const snapshotDir = "src/plugins/discourse/snapshots";
const filename = base64url(url);
const shasum = createHash("sha256");
shasum.update(Buffer.from(((url: any): string), "utf8"));
const filename = shasum.digest("hex");
const file = path.join(snapshotDir, filename);
if (await fs.exists(file)) {
const contents = await fs.readFile(file);
Expand Down

0 comments on commit 47af83f

Please sign in to comment.