Skip to content

Do not read files greater than maxFileSize which is currently 4mb #26169

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

Merged
merged 1 commit into from
Aug 3, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
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
2 changes: 2 additions & 0 deletions src/server/editorServices.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
namespace ts.server {
export const maxProgramSizeForNonTsFiles = 20 * 1024 * 1024;
/*@internal*/
export const maxFileSize = 4 * 1024 * 1024;

// tslint:disable variable-name
export const ProjectsUpdatedInBackgroundEvent = "projectsUpdatedInBackground";
Expand Down
5 changes: 4 additions & 1 deletion src/server/scriptInfo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,10 @@ namespace ts.server {
}

private getFileText(tempFileName?: string) {
return this.host.readFile(tempFileName || this.fileName) || "";
let text: string;
const getText = () => text === undefined ? (text = this.host.readFile(tempFileName || this.fileName) || "") : text;
const size = this.host.getFileSize ? this.host.getFileSize(tempFileName || this.fileName) : getText().length;
return size > maxFileSize ? "" : getText();
}

private switchToScriptVersionCache(): ScriptVersionCache {
Expand Down
47 changes: 46 additions & 1 deletion src/testRunner/unittests/tsserverProjectSystem.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9020,6 +9020,51 @@ export const x = 10;`
});
});

describe("tsserverProjectSystem with large file", () => {
const projectRoot = "/user/username/projects/project";
const largeFile: File = {
path: `${projectRoot}/src/large.ts`,
content: "export var x = 10;",
fileSize: server.maxFileSize + 1
};

it("when large file is included by tsconfig", () => {
const file: File = {
path: `${projectRoot}/src/file.ts`,
content: "export var y = 10;"
};
const tsconfig: File = {
path: `${projectRoot}/tsconfig.json`,
content: JSON.stringify({ files: ["src/file.ts", "src/large.ts"] })
};
const files = [file, largeFile, libFile, tsconfig];
const host = createServerHost(files);
const service = createProjectService(host);
service.openClientFile(file.path);
service.checkNumberOfProjects({ configuredProjects: 1 });
const project = service.configuredProjects.get(tsconfig.path)!;
checkProjectActualFiles(project, [file.path, libFile.path, largeFile.path, tsconfig.path]);
const info = service.getScriptInfo(largeFile.path)!;
assert.equal(info.cacheSourceFile.sourceFile.text, "");
});

it("when large file is included by module resolution", () => {
const file: File = {
path: `${projectRoot}/src/file.ts`,
content: `export var y = 10;import {x} from "./large"`
};
const files = [file, largeFile, libFile];
const host = createServerHost(files);
const service = createProjectService(host);
service.openClientFile(file.path);
service.checkNumberOfProjects({ inferredProjects: 1 });
const project = service.inferredProjects[0];
checkProjectActualFiles(project, [file.path, libFile.path, largeFile.path]);
const info = service.getScriptInfo(largeFile.path)!;
assert.equal(info.cacheSourceFile.sourceFile.text, "");
});
});

describe("tsserverProjectSystem syntax operations", () => {
function navBarFull(session: TestSession, file: File) {
return JSON.stringify(session.executeCommandSeq<protocol.FileRequest>({
Expand Down Expand Up @@ -9535,7 +9580,7 @@ export function Test2() {
});
});

describe("duplicate packages", () => {
describe("tsserverProjectSystem duplicate packages", () => {
// Tests that 'moduleSpecifiers.ts' will import from the redirecting file, and not from the file it redirects to, if that can provide a global module specifier.
it("works with import fixes", () => {
const packageContent = "export const foo: number;";
Expand Down