Skip to content

Commit

Permalink
Merge pull request #7062 from overleaf/jk-fix-file-type-detection
Browse files Browse the repository at this point in the history
[web] Fix file-type detection for `latexmkrc`

GitOrigin-RevId: d51363d2b7d2b1fcc4b783cb3e91f33ab450abba
  • Loading branch information
June Kelly authored and Copybot committed Mar 24, 2022
1 parent 6061e09 commit b567774
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 32 deletions.
30 changes: 23 additions & 7 deletions services/web/app/src/Features/Uploads/FileTypeManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,22 @@ const FileTypeManager = {

// returns charset as understood by fs.readFile,
getType(name, fsPath, callback) {
if (!_isTextFilename(name)) {
if (!name) {
return callback(
new Error(
'[FileTypeManager] getType requires a non-null "name" parameter'
)
)
}
if (!fsPath) {
return callback(
new Error(
'[FileTypeManager] getType requires a non-null "fsPath" parameter'
)
)
}
const basename = Path.basename(name)
if (!_isTextFilename(basename)) {
return callback(null, { binary: true })
}

Expand Down Expand Up @@ -77,7 +92,8 @@ const FileTypeManager = {
},

getStrictTypeFromContent(name, contents) {
const isText = _isTextFilename(name)
const basename = Path.basename(name)
const isText = _isTextFilename(basename)

if (!isText) {
return false
Expand All @@ -98,16 +114,16 @@ const FileTypeManager = {
},

shouldIgnore(path, callback) {
const name = Path.basename(path)
const extension = Path.extname(name).toLowerCase()
const basename = Path.basename(path)
const extension = Path.extname(basename).toLowerCase()
let ignore = false
if (name.startsWith('.') && name !== '.latexmkrc') {
if (basename.startsWith('.') && basename !== '.latexmkrc') {
ignore = true
}
if (FileTypeManager.IGNORE_EXTENSIONS.includes(extension)) {
ignore = true
}
if (FileTypeManager.IGNORE_FILENAMES.includes(name)) {
if (FileTypeManager.IGNORE_FILENAMES.includes(basename)) {
ignore = true
}
callback(null, ignore)
Expand All @@ -118,7 +134,7 @@ function _isTextFilename(filename) {
const extension = Path.extname(filename).toLowerCase()
return (
FileTypeManager.TEXT_EXTENSIONS.includes(extension) ||
filename === 'latexmkrc'
filename.match(/^(\.)?latexmkrc$/)
)
}

Expand Down
58 changes: 33 additions & 25 deletions services/web/test/unit/src/Uploads/FileTypeManagerTests.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,23 +76,25 @@ describe('FileTypeManager', function () {
describe('getType', function () {
describe('when the file extension is text', function () {
const TEXT_FILENAMES = [
'file.tex',
'file.bib',
'file.bibtex',
'file.cls',
'file.bst',
'.latexmkrc',
'latexmkrc',
'file.lbx',
'file.bbx',
'file.cbx',
'file.m',
'file.TEX',
'/file.tex',
'/file.bib',
'/file.bibtex',
'/file.cls',
'/file.bst',
'/.latexmkrc',
'/latexmkrc',
'/file.lbx',
'/other/file.lbx',
'/file.bbx',
'/file.cbx',
'/file.m',
'/something/file.m',
'/file.TEX',
]
TEXT_FILENAMES.forEach(filename => {
it(`should classify ${filename} as text`, function (done) {
this.FileTypeManager.getType(
'file.tex',
filename,
'utf8.tex',
(err, { binary }) => {
if (err) {
Expand All @@ -108,7 +110,7 @@ describe('FileTypeManager', function () {
it('should classify large text files as binary', function (done) {
this.stats.size = 2 * 1024 * 1024 // 2Mb
this.FileTypeManager.getType(
'file.tex',
'/file.tex',
'utf8.tex',
(err, { binary }) => {
if (err) {
Expand All @@ -122,7 +124,7 @@ describe('FileTypeManager', function () {

it('should not try to determine the encoding of large files', function (done) {
this.stats.size = 2 * 1024 * 1024 // 2Mb
this.FileTypeManager.getType('file.tex', 'utf8.tex', err => {
this.FileTypeManager.getType('/file.tex', 'utf8.tex', err => {
if (err) {
return done(err)
}
Expand All @@ -133,7 +135,7 @@ describe('FileTypeManager', function () {

it('should detect the encoding of a utf8 file', function (done) {
this.FileTypeManager.getType(
'file.tex',
'/file.tex',
'utf8.tex',
(err, { binary, encoding }) => {
if (err) {
Expand All @@ -149,7 +151,7 @@ describe('FileTypeManager', function () {

it("should return 'latin1' for non-unicode encodings", function (done) {
this.FileTypeManager.getType(
'file.tex',
'/file.tex',
'latin1.tex',
(err, { binary, encoding }) => {
if (err) {
Expand All @@ -165,7 +167,7 @@ describe('FileTypeManager', function () {

it('should classify utf16 with BOM as utf-16', function (done) {
this.FileTypeManager.getType(
'file.tex',
'/file.tex',
'utf16.tex',
(err, { binary, encoding }) => {
if (err) {
Expand All @@ -181,7 +183,7 @@ describe('FileTypeManager', function () {

it('should classify latin1 files with a null char as binary', function (done) {
this.FileTypeManager.getType(
'file.tex',
'/file.tex',
'latin1-null.tex',
(err, { binary }) => {
if (err) {
Expand All @@ -195,7 +197,7 @@ describe('FileTypeManager', function () {

it('should classify utf8 files with a null char as binary', function (done) {
this.FileTypeManager.getType(
'file.tex',
'/file.tex',
'utf8-null.tex',
(err, { binary }) => {
if (err) {
Expand All @@ -209,7 +211,7 @@ describe('FileTypeManager', function () {

it('should classify utf8 files with non-BMP chars as binary', function (done) {
this.FileTypeManager.getType(
'file.tex',
'/file.tex',
'utf8-non-bmp.tex',
(err, { binary }) => {
if (err) {
Expand All @@ -223,7 +225,7 @@ describe('FileTypeManager', function () {

it('should classify utf8 files with ascii control chars as utf-8', function (done) {
this.FileTypeManager.getType(
'file.tex',
'/file.tex',
'utf8-control-chars.tex',
(err, { binary, encoding }) => {
if (err) {
Expand All @@ -238,11 +240,17 @@ describe('FileTypeManager', function () {
})

describe('when the file extension is non-text', function () {
const BINARY_FILENAMES = ['file.eps', 'file.dvi', 'file.png', 'tex']
const BINARY_FILENAMES = [
'/file.eps',
'/file.dvi',
'/file.png',
'/images/file.png',
'/tex',
]
BINARY_FILENAMES.forEach(filename => {
it(`should classify ${filename} as binary`, function (done) {
this.FileTypeManager.getType(
'file.tex',
'/file.tex',
'utf8.tex',
(err, { binary }) => {
if (err) {
Expand All @@ -256,7 +264,7 @@ describe('FileTypeManager', function () {
})

it('should not try to get the character encoding', function (done) {
this.FileTypeManager.getType('file.png', 'utf8.tex', err => {
this.FileTypeManager.getType('/file.png', 'utf8.tex', err => {
if (err) {
return done(err)
}
Expand Down

0 comments on commit b567774

Please sign in to comment.