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

[api-minor] Improve the FileSpec implementation #18034

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
4 changes: 2 additions & 2 deletions src/core/catalog.js
Original file line number Diff line number Diff line change
Expand Up @@ -1619,8 +1619,8 @@ class Catalog {
/* xref = */ null,
/* skipContent = */ true
);
const { filename } = fs.serializable;
url = filename;
const { rawFilename } = fs.serializable;
url = rawFilename;
} else if (typeof urlDict === "string") {
url = urlDict;
}
Expand Down
31 changes: 20 additions & 11 deletions src/core/file_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@
import { Dict } from "./primitives.js";

function pickPlatformItem(dict) {
if (!(dict instanceof Dict)) {
return null;
}
// Look for the filename in this order:
// UF, F, Unix, Mac, DOS
if (dict.has("UF")) {
Expand All @@ -34,6 +37,10 @@
return null;
}

function stripPath(str) {
return str.substring(str.lastIndexOf("/") + 1);
}

/**
* "A PDF file can refer to the contents of another file by using a File
* Specification (PDF 1.1)", see the spec (7.11) for more details.
Expand Down Expand Up @@ -66,26 +73,27 @@
}

get filename() {
if (!this._filename && this.root) {
const filename = pickPlatformItem(this.root) || "unnamed";
this._filename = stringToPDFString(filename)
let filename = "";

const item = pickPlatformItem(this.root);
if (item && typeof item === "string") {
filename = stringToPDFString(item)
.replaceAll("\\\\", "\\")

Check failure

Code scanning / CodeQL

Double escaping or unescaping High

This replacement may produce '' characters that are double-unescaped
here
.
.replaceAll("\\/", "/")
.replaceAll("\\", "/");
}
return this._filename;
return shadow(this, "filename", filename || "unnamed");
}

get content() {
if (!this.#contentAvailable) {
return null;
}
if (!this.contentRef && this.root) {
this.contentRef = pickPlatformItem(this.root.get("EF"));
}
this._contentRef ||= pickPlatformItem(this.root?.get("EF"));

let content = null;
if (this.contentRef) {
const fileObj = this.xref.fetchIfRef(this.contentRef);
if (this._contentRef) {
const fileObj = this.xref.fetchIfRef(this._contentRef);
if (fileObj instanceof BaseStream) {
content = fileObj.getBytes();
} else {
Expand All @@ -94,7 +102,7 @@
);
}
} else {
warn("Embedded file specification does not have a content");
warn("Embedded file specification does not have any content");
}
return content;
}
Expand All @@ -111,7 +119,8 @@

get serializable() {
return {
filename: this.filename,
rawFilename: this.filename,
filename: stripPath(this.filename),
content: this.content,
description: this.description,
};
Expand Down
11 changes: 4 additions & 7 deletions src/display/annotation_layer.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ import {
} from "../shared/util.js";
import {
DOMSVGFactory,
getFilenameFromUrl,
PDFDateString,
setLayerDimensions,
} from "./display_utils.js";
Expand Down Expand Up @@ -2859,15 +2858,13 @@ class FileAttachmentAnnotationElement extends AnnotationElement {
constructor(parameters) {
super(parameters, { isRenderable: true });

const { filename, content, description } = this.data.file;
this.filename = getFilenameFromUrl(filename, /* onlyStripPath = */ true);
this.content = content;
const { file } = this.data;
this.filename = file.filename;
this.content = file.content;

this.linkService.eventBus?.dispatch("fileattachmentannotation", {
source: this,
filename,
content,
description,
...file,
});
}

Expand Down
9 changes: 6 additions & 3 deletions test/unit/annotation_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -4033,9 +4033,12 @@ describe("annotation", function () {
idFactoryMock
);
expect(data.annotationType).toEqual(AnnotationType.FILEATTACHMENT);
expect(data.file.filename).toEqual("Test.txt");
expect(data.file.content).toEqual(stringToBytes("Test attachment"));
expect(data.file.description).toEqual("abc");
expect(data.file).toEqual({
rawFilename: "Test.txt",
filename: "Test.txt",
content: stringToBytes("Test attachment"),
description: "abc",
});
});
});

Expand Down
16 changes: 9 additions & 7 deletions test/unit/api_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -1475,12 +1475,12 @@ describe("api", function () {
const pdfDoc = await loadingTask.promise;
const attachments = await pdfDoc.getAttachments();

const { filename, content, description } = attachments["foo.txt"];
expect(filename).toEqual("foo.txt");
expect(content).toEqual(
new Uint8Array([98, 97, 114, 32, 98, 97, 122, 32, 10])
);
expect(description).toEqual("");
expect(attachments["foo.txt"]).toEqual({
rawFilename: "foo.txt",
filename: "foo.txt",
content: new Uint8Array([98, 97, 114, 32, 98, 97, 122, 32, 10]),
description: "",
});

await loadingTask.destroy();
});
Expand All @@ -1490,7 +1490,9 @@ describe("api", function () {
const pdfDoc = await loadingTask.promise;
const attachments = await pdfDoc.getAttachments();

const { filename, content, description } = attachments["empty.pdf"];
const { rawFilename, filename, content, description } =
attachments["empty.pdf"];
expect(rawFilename).toEqual("Empty page.pdf");
expect(filename).toEqual("Empty page.pdf");
expect(content instanceof Uint8Array).toEqual(true);
expect(content.length).toEqual(2357);
Expand Down
22 changes: 6 additions & 16 deletions web/pdf_attachment_viewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
/** @typedef {import("./download_manager.js").DownloadManager} DownloadManager */

import { BaseTreeViewer } from "./base_tree_viewer.js";
import { getFilenameFromUrl } from "pdfjs-lib";
import { waitOnEventOrTimeout } from "./event_utils.js";

/**
Expand Down Expand Up @@ -122,19 +121,13 @@ class PDFAttachmentViewer extends BaseTreeViewer {
let attachmentsCount = 0;
for (const name in attachments) {
const item = attachments[name];
const content = item.content,
description = item.description,
filename = getFilenameFromUrl(
item.filename,
/* onlyStripPath = */ true
Snuffleupagus marked this conversation as resolved.
Show resolved Hide resolved
);

const div = document.createElement("div");
div.className = "treeItem";

const element = document.createElement("a");
this._bindLink(element, { content, description, filename });
element.textContent = this._normalizeTextContent(filename);
this._bindLink(element, item);
element.textContent = this._normalizeTextContent(item.filename);

div.append(element);

Expand All @@ -148,7 +141,7 @@ class PDFAttachmentViewer extends BaseTreeViewer {
/**
* Used to append FileAttachment annotations to the sidebar.
*/
#appendAttachment({ filename, content, description }) {
#appendAttachment(item) {
const renderedPromise = this._renderedCapability.promise;

renderedPromise.then(() => {
Expand All @@ -158,15 +151,12 @@ class PDFAttachmentViewer extends BaseTreeViewer {
const attachments = this._attachments || Object.create(null);

for (const name in attachments) {
if (filename === name) {
if (item.filename === name) {
return; // Ignore the new attachment if it already exists.
}
}
attachments[filename] = {
filename,
content,
description,
};
attachments[item.filename] = item;

this.render({
attachments,
keepRenderedCapability: true,
Expand Down
Loading