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: Allow explicit null encoding for readFile/writeFile #18534

Merged
merged 16 commits into from
Oct 26, 2021
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion cli/types/cypress.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5672,7 +5672,7 @@ declare namespace Cypress {
xhr: XMLHttpRequest
}

type Encodings = 'ascii' | 'base64' | 'binary' | 'hex' | 'latin1' | 'utf8' | 'utf-8' | 'ucs2' | 'ucs-2' | 'utf16le' | 'utf-16le'
type Encodings = 'ascii' | 'base64' | 'binary' | 'hex' | 'latin1' | 'utf8' | 'utf-8' | 'ucs2' | 'ucs-2' | 'utf16le' | 'utf-16le' | null
type PositionType = 'topLeft' | 'top' | 'topRight' | 'left' | 'center' | 'right' | 'bottomLeft' | 'bottom' | 'bottomRight'
type ViewportPreset = 'macbook-16' | 'macbook-15' | 'macbook-13' | 'macbook-11' | 'ipad-2' | 'ipad-mini' | 'iphone-xr' | 'iphone-x' | 'iphone-6+' | 'iphone-se2' | 'iphone-8' | 'iphone-7' | 'iphone-6' | 'iphone-5' | 'iphone-4' | 'iphone-3' | 'samsung-s10' | 'samsung-note9'
interface Offset {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
const { $, _ } = Cypress

describe('src/cy/commands/assertions', () => {
describe('src/cy/commands/assertions', { retries: 2 }, () => {
BlueWinds marked this conversation as resolved.
Show resolved Hide resolved
before(() => {
cy
.visit('/fixtures/jquery.html')
Expand Down
33 changes: 33 additions & 0 deletions packages/driver/cypress/integration/commands/files_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,22 @@ describe('src/cy/commands/files', () => {
})
})

// https://github.com/cypress-io/cypress/issues/1558
it('passes explicit null encoding through to server and decodes response', () => {
BlueWinds marked this conversation as resolved.
Show resolved Hide resolved
Cypress.backend.resolves({
contents: Buffer.from('\n').toString('base64'),
filePath: '/path/to/foo.json',
})

cy.readFile('foo.json', null).then(() => {
expect(Cypress.backend).to.be.calledWith(
'read:file',
'foo.json',
{ encoding: null },
)
}).should('eql', Buffer.from('\n'))
})

it('sets the contents as the subject', () => {
Cypress.backend.resolves(okResponse)

Expand Down Expand Up @@ -340,6 +356,23 @@ describe('src/cy/commands/files', () => {
})
})

// https://github.com/cypress-io/cypress/issues/1558
it('explicit null encoding is sent to server as base64 string', () => {
Cypress.backend.resolves(okResponse)

cy.writeFile('foo.txt', Buffer.from([0, 0, 54, 255]), null).then(() => {
expect(Cypress.backend).to.be.calledWith(
'write:file',
'foo.txt',
'AAA2/w==',
{
encoding: 'base64',
flag: 'w',
},
)
})
})

it('can take encoding as part of options', () => {
Cypress.backend.resolves(okResponse)

Expand Down
11 changes: 11 additions & 0 deletions packages/driver/cypress/integration/commands/fixtures_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,17 @@ describe('src/cy/commands/fixtures', () => {
})
})

// https://github.com/cypress-io/cypress/issues/1558
it('passes explicit null encoding through to server and decodes response', () => {
BlueWinds marked this conversation as resolved.
Show resolved Hide resolved
Cypress.backend.withArgs('get:fixture').resolves(Buffer.from('\n').toString('base64'))

cy.fixture('foo', null).then((obj) => {
expect(Cypress.backend).to.be.calledWith('get:fixture', 'foo', {
encoding: null,
})
}).should('eql', Buffer.from('\n'))
})

it('can have encoding as second argument and options as third argument', () => {
Cypress.backend.withArgs('get:fixture').resolves({ foo: 'bar' })

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ const helpers = require('../../support/helpers')

const { _, Promise, $ } = Cypress

describe('src/cy/commands/navigation', () => {
describe('src/cy/commands/navigation', { retries: 2 }, () => {
BlueWinds marked this conversation as resolved.
Show resolved Hide resolved
context('#reload', () => {
before(() => {
cy
Expand Down
29 changes: 25 additions & 4 deletions packages/driver/src/cy/commands/files.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,16 @@ export default (Commands, Cypress, cy) => {

if (_.isObject(encoding)) {
userOptions = encoding
encoding = null
encoding = undefined
BlueWinds marked this conversation as resolved.
Show resolved Hide resolved
}

options = _.defaults({}, userOptions, {
encoding: encoding != null ? encoding : 'utf8',
// https://github.com/cypress-io/cypress/issues/1558
// If no encoding is specified, then Cypress has historically defaulted
// to `utf8`, because of it's focus on text files. This is in contrast to
// NodeJs, which defaults to binary. We allow users to pass in `null`
// to restore the default node behavior.
encoding: encoding === undefined ? 'utf8' : encoding,
flotwig marked this conversation as resolved.
Show resolved Hide resolved
log: true,
})

Expand Down Expand Up @@ -53,6 +58,12 @@ export default (Commands, Cypress, cy) => {
args: { cmd: 'readFile', action: 'read', file, filePath: err.filePath, error: err.message },
})
}).then(({ contents, filePath }) => {
// Binary files (read with explicit `null` encoding by the user) are transmitted over the
// websocket base64 encoded. See packages/server/lib/files.js.
if (options.encoding === null) {
contents = Buffer.from(contents, 'base64')
}

consoleProps['File Path'] = filePath
consoleProps['Contents'] = contents

Expand Down Expand Up @@ -85,11 +96,16 @@ export default (Commands, Cypress, cy) => {

if (_.isObject(encoding)) {
userOptions = encoding
encoding = null
encoding = undefined
}

options = _.defaults({}, userOptions, {
encoding: encoding ? encoding : 'utf8',
// https://github.com/cypress-io/cypress/issues/1558
// If no encoding is specified, then Cypress has historically defaulted
// to `utf8`, because of it's focus on text files. This is in contrast to
// NodeJs, which defaults to binary. We allow users to pass in `null`
// to restore the default node behavior.
encoding: encoding === undefined ? 'utf8' : encoding,
flag: userOptions.flag ? userOptions.flag : 'w',
log: true,
})
Expand Down Expand Up @@ -120,6 +136,11 @@ export default (Commands, Cypress, cy) => {
})
}

if (Buffer.isBuffer(contents)) {
contents = contents.toString('base64')
options.encoding = 'base64'
}

if (_.isObject(contents)) {
contents = JSON.stringify(contents, null, 2)
}
Expand Down
13 changes: 12 additions & 1 deletion packages/driver/src/cy/commands/fixtures.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ import Promise from 'bluebird'
import $errUtils from '../../cypress/error_utils'

const clone = (obj) => {
if (Buffer.isBuffer(obj)) {
return Buffer.from(obj)
}

return JSON.parse(JSON.stringify(obj))
}

Expand Down Expand Up @@ -47,7 +51,7 @@ export default (Commands, Cypress, cy, state, config) => {
options = args[1]
}

if (_.isString(args[0])) {
if (_.isString(args[0]) || args[0] === null) {
options.encoding = args[0]
}

Expand All @@ -64,6 +68,13 @@ export default (Commands, Cypress, cy, state, config) => {
return $errUtils.throwErr(response.__error)
}

// https://github.com/cypress-io/cypress/issues/1558
// Binary files (read with explicit `null` encoding by the user) are transmitted over the
// websocket base64 encoded. See packages/server/lib/fixture.js.
if (options.encoding === null) {
response = Buffer.from(response, 'base64')
}

// add the fixture to the cache
// so it can just be returned next time
cache[fixture] = response
Expand Down
2 changes: 1 addition & 1 deletion packages/net-stubbing/lib/server/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ export async function setResponseFromFixture (getFixtureFn: GetFixtureFn, static
return
}

const data = await getFixtureFn(fixture.filePath, { encoding: fixture.encoding || null })
const data = await getFixtureFn(fixture.filePath, { encoding: fixture.encoding })

const { headers } = staticResponse

Expand Down
13 changes: 10 additions & 3 deletions packages/server/lib/files.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,17 @@ module.exports = {
const filePath = path.resolve(projectRoot, file)
const readFn = path.extname(filePath) === '.json' ? fs.readJsonAsync : fs.readFileAsync

return readFn(filePath, options.encoding || 'utf8')
// https://github.com/cypress-io/cypress/issues/1558
// If no encoding is specified, then Cypress has historically defaulted
// to `utf8`, because of it's focus on text files. This is in contrast to
// NodeJs, which defaults to binary. We allow users to pass in `null`
// to restore the default node behavior.
return readFn(filePath, options.encoding === undefined ? 'utf8' : options.encoding)
.then((contents) => {
return {
contents,
// For binary files, we base64 encode them for transmission over the websocket
flotwig marked this conversation as resolved.
Show resolved Hide resolved
// There is a matching Buffer.from() in packages/driver/src/cy/commands/files.ts
contents: options.encoding === null ? contents.toString('base64') : contents,
filePath,
}
})
Expand All @@ -22,7 +29,7 @@ module.exports = {
writeFile (projectRoot, file, contents, options = {}) {
const filePath = path.resolve(projectRoot, file)
const writeOptions = {
encoding: options.encoding || 'utf8',
encoding: options.encoding === undefined ? 'utf8' : options.encoding,
flag: options.flag || 'w',
}

Expand Down
16 changes: 12 additions & 4 deletions packages/server/lib/fixture.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
const _ = require('lodash')
const path = require('path')
const check = require('syntax-error')
const debug = require('debug')('cypress:server:fixture')
Expand Down Expand Up @@ -116,15 +115,24 @@ module.exports = {
},

parseFileByExtension (p, fixture, ext, options = {}) {
// https://github.com/cypress-io/cypress/issues/1558
// If the user explicitly specifies `null` as the encoding, we treat the
// file as binary regardless of extension. We base64 encode them for
// transmission over the websocket. There is a matching Buffer.from()
// in packages/driver/src/cy/commands/fixtures.ts
if (options.encoding === null) {
return this.parse(p, fixture)
}

switch (ext) {
case '.json': return this.parseJson(p, fixture)
case '.js': return this.parseJs(p, fixture)
case '.coffee': return this.parseCoffee(p, fixture)
case '.html': return this.parseHtml(p, fixture)
case '.png': case '.jpg': case '.jpeg': case '.gif': case '.tif': case '.tiff': case '.zip':
return this.parse(p, fixture, _.isNull(options.encoding) ? null : (options.encoding || 'base64'))
return this.parse(p, fixture, options.encoding)
default:
return this.parse(p, fixture, _.isNull(options.encoding) ? null : options.encoding)
return this.parse(p, fixture, options.encoding || 'utf8')
}
},

Expand Down Expand Up @@ -184,7 +192,7 @@ module.exports = {
.bind(this)
},

parse (p, fixture, encoding = 'utf8') {
parse (p, fixture, encoding) {
return fs.readFileAsync(p, encoding)
.bind(this)
},
Expand Down
4 changes: 4 additions & 0 deletions packages/server/lib/socket-base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -375,6 +375,10 @@ export class SocketBase {
return firefoxUtil.windowFocus()
case 'get:fixture':
return getFixture(args[0], args[1])
// Buffers are base64 encoded for sending over the websocket. This is done
// here rather than in fixture code because net stubbing wants to send the
// unencoded buffer, and it also relies on the same fixture loading code.
.then((resp) => Buffer.isBuffer(resp) ? resp.toString('base64') : resp)
case 'read:file':
return files.readFile(config.projectRoot, args[0], args[1])
case 'write:file':
Expand Down
16 changes: 16 additions & 0 deletions packages/server/test/unit/files_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,13 @@ describe('lib/files', () => {
})
})

// https://github.com/cypress-io/cypress/issues/1558
it('explicit null encoding is sent to driver as base64 string', function () {
return files.readFile(this.projectRoot, 'tests/_fixtures/ascii.foo', { encoding: null }).then(({ contents }) => {
expect(contents).to.eql(Buffer.from('\n').toString('base64'))
})
})

it('parses json to valid JS object', function () {
return files.readFile(this.projectRoot, 'tests/_fixtures/users.json').then(({ contents }) => {
expect(contents).to.eql([
Expand Down Expand Up @@ -75,6 +82,15 @@ describe('lib/files', () => {
})
})

// https://github.com/cypress-io/cypress/issues/1558
it('explicit null encoding is read from driver as base64 string', function () {
return files.writeFile(this.projectRoot, '.projects/write_file.txt', Buffer.from(''), { encoding: null }).then(() => {
return files.readFile(this.projectRoot, '.projects/write_file.txt', { encoding: null }).then(({ contents }) => {
expect(contents).to.eql(Buffer.from('').toString('base64'))
})
})
})

it('overwrites existing file by default', function () {
return files.writeFile(this.projectRoot, '.projects/write_file.txt', 'foo').then(() => {
return files.readFile(this.projectRoot, '.projects/write_file.txt').then(({ contents }) => {
Expand Down