Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(docs): allow audio (mp3/ogg), video (mp4/webm) and font (woff2) attachments #7605

Merged
merged 26 commits into from
Jun 2, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
fef26a9
chore(build,content,server): allow mp3/mp4/ogg/ttf/webm attachments
caugner Nov 16, 2022
034b2f8
chore(file-attachment): type params
caugner Nov 17, 2022
bd11bf5
fix(file-attachment): always check for directories
caugner Nov 17, 2022
32e8446
chore(file-attachment): restrict images to file types in content
caugner Nov 17, 2022
c05538b
chore(file-attachment): support woff/woff2
caugner Nov 18, 2022
6c47673
chore(build): check file-type of audio/video/font files
caugner Nov 18, 2022
1979be7
fix(constants): add mp3/mp4/ogg/ttf/webm to VALID_MIME_TYPES
caugner Nov 25, 2022
367d0a8
refactor(build): move file extension check into filecheck
caugner Nov 28, 2022
22e1977
refactor(filecheck): extract checkCompression()
caugner Nov 28, 2022
ab21cd3
fix(filecheck): ignore compression of other files
caugner Nov 28, 2022
864bbcc
chore: remove TTF support
caugner Nov 28, 2022
fb90319
chore(build,filecheck): add param types
caugner Dec 7, 2022
eacc2da
fix(filecheck): check extension of ogg files
caugner Dec 9, 2022
023c6ad
fix(filecheck): reuse sizeBefore
caugner Dec 9, 2022
357ddb7
chore(file-attachment): only allow woff2, not woff
caugner Dec 16, 2022
c024872
fix(cloud-function): update asset extensions
caugner May 11, 2023
9ebc27f
refactor(libs/constants): extract extension constants
caugner May 11, 2023
33ec69f
test(libs/constants): add test cases + new test:lib script
caugner May 11, 2023
c24bd17
chore(filecheck/checker): make binary file check clearer
caugner May 11, 2023
037ac80
feat(docs): allow avif video assets
caugner May 11, 2023
ae818db
chore(build): update some comments
caugner May 11, 2023
c22d47d
Revert "feat(docs): allow avif video assets"
caugner May 12, 2023
19944c7
fix(webpack): declare audio/video/font files as modules
caugner May 29, 2023
04f2d16
fix(client): update proxied extensions for dev server
caugner Jun 2, 2023
cddc538
chore: update comments
caugner Jun 2, 2023
7e3326a
chore(libs/constants): remove font/woff
caugner Jun 2, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .github/workflows/testing.yml
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,9 @@ jobs:
- name: Unit testing client
run: yarn test:client

- name: Unit testing libs
run: yarn test:libs

- name: Build and start server
id: server
env:
Expand Down
12 changes: 6 additions & 6 deletions build/check-images.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import path from "node:path";

import imagesize from "image-size";

import { Document, Image } from "../content/index.js";
import { Document, FileAttachment } from "../content/index.js";
import { FLAW_LEVELS, DEFAULT_LOCALE } from "../libs/constants/index.js";
import { findMatchesInText } from "./matches-in-text.js";
import * as cheerio from "cheerio";
Expand Down Expand Up @@ -140,12 +140,12 @@ export function checkImageReferences(
// but all our images are going to be static.
finalSrc = absoluteURL.pathname;
// We can use the `finalSrc` to look up and find the image independent
// of the correct case because `Image.findByURL` operates case
// of the correct case because `FileAttachment.findByURL` operates case
// insensitively.

// What follows uses the same algorithm as Image.findByURLWithFallback
// What follows uses the same algorithm as FileAttachment.findByURLWithFallback
// but only adds a filePath if it exists for the DEFAULT_LOCALE
const filePath = Image.findByURL(finalSrc);
const filePath = FileAttachment.findByURL(finalSrc);
let enUSFallback = false;
if (
!filePath &&
Expand All @@ -156,7 +156,7 @@ export function checkImageReferences(
new RegExp(`^/${doc.locale}/`, "i"),
`/${DEFAULT_LOCALE}/`
);
if (Image.findByURL(enUSFinalSrc)) {
if (FileAttachment.findByURL(enUSFinalSrc)) {
// Use the en-US src instead
finalSrc = enUSFinalSrc;
// Note that this `<img src="...">` value can work if you use the
Expand Down Expand Up @@ -366,7 +366,7 @@ export function checkImageWidths(
);
}
} else if (!imgSrc.includes("://") && imgSrc.startsWith("/")) {
const filePath = Image.findByURLWithFallback(imgSrc);
const filePath = FileAttachment.findByURLWithFallback(imgSrc);
if (filePath) {
const dimensions = sizeOf(filePath);
img.attr("width", `${dimensions.width}`);
Expand Down
6 changes: 3 additions & 3 deletions build/flaws/broken-links.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import fs from "node:fs";
import { fromMarkdown } from "mdast-util-from-markdown";
import { visit } from "unist-util-visit";

import { Document, Redirect, Image } from "../../content/index.js";
import { Document, Redirect, FileAttachment } from "../../content/index.js";
import { findMatchesInText } from "../matches-in-text.js";
import {
DEFAULT_LOCALE,
Expand Down Expand Up @@ -277,8 +277,8 @@ export function getBrokenLinksFlaws(
const absoluteURL = new URL(href, "http://www.example.com");
const found = Document.findByURL(hrefNormalized);
if (!found) {
// Before we give up, check if it's an image.
if (!Image.findByURLWithFallback(hrefNormalized)) {
// Before we give up, check if it's an attachment.
if (!FileAttachment.findByURLWithFallback(hrefNormalized)) {
// Even if it's a redirect, it's still a flaw, but it'll be nice to
// know what it *should* be.
const resolved = Redirect.resolve(hrefNormalized);
Expand Down
6 changes: 3 additions & 3 deletions build/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ import LANGUAGES_RAW from "../libs/languages/index.js";
import { safeDecodeURIComponent } from "../kumascript/src/api/util.js";
import { wrapTables } from "./wrap-tables.js";
import {
getAdjacentImages,
getAdjacentFileAttachments,
injectLoadingLazyAttributes,
injectNoTranslate,
makeTOC,
Expand Down Expand Up @@ -382,8 +382,8 @@ export async function buildDocument(
// The checkImageReferences() does 2 things. Checks image *references* and
// it returns which images it checked. But we'll need to complement any
// other images in the folder.
getAdjacentImages(path.dirname(document.fileInfo.path)).forEach((fp) =>
fileAttachments.add(fp)
getAdjacentFileAttachments(path.dirname(document.fileInfo.path)).forEach(
(fp) => fileAttachments.add(fp)
);

// Check the img tags for possible flaws and possible build-time rewrites
Expand Down
26 changes: 13 additions & 13 deletions build/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,11 @@ import imageminSvgo from "imagemin-svgo";
import { rgPath } from "@vscode/ripgrep";
import sanitizeFilename from "sanitize-filename";

import { VALID_MIME_TYPES } from "../libs/constants/index.js";
import { Image } from "../content/index.js";
import {
ANY_ATTACHMENT_REGEXP,
VALID_MIME_TYPES,
} from "../libs/constants/index.js";
import { FileAttachment } from "../content/index.js";
import { spawnSync } from "node:child_process";
import { BLOG_ROOT } from "../libs/env/index.js";

Expand Down Expand Up @@ -184,15 +187,12 @@ export function splitSections(rawHTML) {
*
* @param {Document} document
*/
export function getAdjacentImages(documentDirectory) {
export function getAdjacentFileAttachments(documentDirectory: string) {
const dirents = fs.readdirSync(documentDirectory, { withFileTypes: true });
return dirents
.filter((dirent) => {
// This needs to match what we do in filecheck/checker.py
return (
!dirent.isDirectory() &&
/\.(png|jpeg|jpg|gif|svg|webp)$/i.test(dirent.name)
);
return !dirent.isDirectory() && ANY_ATTACHMENT_REGEXP.test(dirent.name);
})
.map((dirent) => path.join(documentDirectory, dirent.name));
}
Expand Down Expand Up @@ -249,21 +249,21 @@ export function postLocalFileLinks($, doc) {
const href = element.attribs.href;

// This test is merely here to quickly bail if there's no hope to find the
// image as a local file link. There are a LOT of hyperlinks throughout
// the content and this simple if statement means we can skip 99% of the
// links, so it's presumed to be worth it.
// file attachment as a local file link. There are a LOT of hyperlinks
// throughout the content and this simple if statement means we can skip 99%
// of the links, so it's presumed to be worth it.
if (
!href ||
/^(\/|\.\.|http|#|mailto:|about:|ftp:|news:|irc:|ftp:)/i.test(href)
) {
return;
}
// There are a lot of links that don't match. E.g. `<a href="SubPage">`
// So we'll look-up a lot "false positives" that are not images.
// So we'll look-up a lot "false positives" that are not file attachments.
// Thankfully, this lookup is fast.
const url = `${doc.mdn_url}/${href}`;
const image = Image.findByURLWithFallback(url);
if (image) {
const fileAttachment = FileAttachment.findByURLWithFallback(url);
if (fileAttachment) {
$(element).attr("href", url);
}
});
Expand Down
25 changes: 25 additions & 0 deletions client/src/react-app.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,16 +34,41 @@ declare module "*.jpeg" {
export default src;
}

declare module "*.mp3" {
const src: string;
export default src;
}

declare module "*.mp4" {
const src: string;
export default src;
}

declare module "*.ogg" {
const src: string;
export default src;
}

declare module "*.png" {
const src: string;
export default src;
}

declare module "*.webm" {
const src: string;
export default src;
}

declare module "*.webp" {
const src: string;
export default src;
}

declare module "*.woff2" {
const src: string;
export default src;
}

declare module "*.svg" {
import * as React from "react";

Expand Down
4 changes: 2 additions & 2 deletions client/src/setupProxy.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ function config(app) {
app.use("/_+(flaws|translations|open|document)", proxy);
// E.g. search-index.json or index.json
app.use("**/*.json", proxy);
// This has to match what we do in server/index.js in the catchall handler
app.use("**/*.(png|webp|gif|jpe?g|svg)", proxy);
// Always update libs/constant/index.js when adding/removing extensions!
app.use(`**/*.(gif|jpeg|jpg|mp3|mp4|ogg|png|svg|webm|webp|woff2)`, proxy);
// All those root-level images like /favicon-48x48.png
app.use("/*.(png|webp|gif|jpe?g|svg)", proxy);
}
Expand Down
4 changes: 3 additions & 1 deletion cloud-function/src/app.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import express, { Request, Response } from "express";
import { Router } from "express";

import { ANY_ATTACHMENT_EXT } from "./internal/constants/index.js";

import { Origin } from "./env.js";
import { proxyContent } from "./handlers/proxy-content.js";
import { proxyKevel } from "./handlers/proxy-kevel.js";
Expand Down Expand Up @@ -48,7 +50,7 @@ router.get(
proxyContent
);
router.get(
"/[^/]+/docs/*/*.(png|jpeg|jpg|gif|svg|webp)",
`/[^/]+/docs/*/*.(${ANY_ATTACHMENT_EXT.join("|")})`,
requireOrigin(Origin.main, Origin.liveSamples),
resolveIndexHTML,
proxyContent
Expand Down
13 changes: 11 additions & 2 deletions cloud-function/src/utils.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
import { Request, Response } from "express";

import {
ANY_ATTACHMENT_EXT,
createRegExpFromExtensions,
} from "./internal/constants/index.js";

import { DEFAULT_COUNTRY } from "./constants.js";

export function getRequestCountry(req: Request): string {
Expand Down Expand Up @@ -45,8 +50,12 @@ export function isLiveSampleURL(url: string) {

// These are the only extensions in client/build/*/docs/*.
// `find client/build -type f | grep docs | xargs basename | sed 's/.*\.\([^.]*\)$/\1/' | sort | uniq`
const ASSET_REGEXP = /\.(gif|html|jpeg|jpg|json|png|svg|txt|xml)$/i;
const TEXT_EXT = ["html", "json", "svg", "txt", "xml"];
const ANY_ATTACHMENT_REGEXP = createRegExpFromExtensions(
...ANY_ATTACHMENT_EXT,
...TEXT_EXT
);

export function isAsset(url: string) {
return ASSET_REGEXP.test(url);
return ANY_ATTACHMENT_REGEXP.test(url);
}
44 changes: 41 additions & 3 deletions content/image.ts → content/file-attachment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,52 @@ import { readChunkSync } from "read-chunk";
import imageType from "image-type";
import isSvg from "is-svg";

import { DEFAULT_LOCALE } from "../libs/constants/index.js";
import {
ANY_IMAGE_EXT,
AUDIO_EXT,
DEFAULT_LOCALE,
FONT_EXT,
VIDEO_EXT,
createRegExpFromExtensions,
} from "../libs/constants/index.js";
import { ROOTS } from "../libs/env/index.js";
import { memoize, slugToFolder } from "./utils.js";

function isImage(filePath: string) {
function isFileAttachment(filePath: string) {
if (fs.statSync(filePath).isDirectory()) {
return false;
}

return (
isAudio(filePath) ||
isFont(filePath) ||
isVideo(filePath) ||
isImage(filePath)
);
}

const AUDIO_FILE_REGEXP = createRegExpFromExtensions(...AUDIO_EXT);
const FONT_FILE_REGEXP = createRegExpFromExtensions(...FONT_EXT);
const VIDEO_FILE_REGEXP = createRegExpFromExtensions(...VIDEO_EXT);
const IMAGE_FILE_REGEXP = createRegExpFromExtensions(...ANY_IMAGE_EXT);

function isAudio(filePath: string) {
return AUDIO_FILE_REGEXP.test(filePath);
}

function isFont(filePath: string) {
return FONT_FILE_REGEXP.test(filePath);
}

function isVideo(filePath: string) {
return VIDEO_FILE_REGEXP.test(filePath);
}

function isImage(filePath: string) {
if (!IMAGE_FILE_REGEXP.test(filePath)) {
return false;
}

if (filePath.toLowerCase().endsWith(".svg")) {
return isSvg(fs.readFileSync(filePath, "utf-8"));
}
Expand All @@ -37,7 +75,7 @@ function urlToFilePath(url: string) {

const find = memoize((relativePath: string) => {
return ROOTS.map((root) => path.join(root, relativePath)).find(
(filePath) => fs.existsSync(filePath) && isImage(filePath)
(filePath) => fs.existsSync(filePath) && isFileAttachment(filePath)
);
});

Expand Down
2 changes: 1 addition & 1 deletion content/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ export * as Document from "./document.js";
export * as Translation from "./translation.js";
export { getPopularities } from "./popularities.js";
export * as Redirect from "./redirect.js";
export * as Image from "./image.js";
export * as FileAttachment from "./file-attachment.js";
export {
buildURL,
memoize,
Expand Down
Loading