diff --git a/lib/Repository.js b/lib/Repository.js index 74452dde..7823b099 100644 --- a/lib/Repository.js +++ b/lib/Repository.js @@ -231,13 +231,13 @@ class Repository extends Requestable { * Get tha sha for a particular object in the repository. This is a convenience function * @see https://developer.github.com/v3/repos/contents/#get-contents * @param {string} [branch] - the branch to look in, or the repository's default branch if omitted - * @param {string} path - the path of the file or directory + * @param {string} path - the unencoded path of the file or directory * @param {Requestable.callback} cb - will receive a description of the requested object, including a `SHA` property * @return {Promise} - the promise for the http request */ getSha(branch, path, cb) { branch = branch ? `?ref=${branch}` : ''; - return this._request('GET', `/repos/${this.__fullname}/contents/${path}${branch}`, null, cb); + return this._request('GET', `/repos/${this.__fullname}/contents/${encodeContentPath(path)}${branch}`, null, cb); } /** @@ -499,13 +499,13 @@ class Repository extends Requestable { * Get the contents of a repository * @see https://developer.github.com/v3/repos/contents/#get-contents * @param {string} ref - the ref to check - * @param {string} path - the path containing the content to fetch + * @param {string} path - the unencoded path containing the content to fetch * @param {boolean} raw - `true` if the results should be returned raw instead of GitHub's normalized format * @param {Requestable.callback} cb - will receive the fetched data * @return {Promise} - the promise for the http request */ getContents(ref, path, raw, cb) { - path = path ? `${encodeURI(path)}` : ''; + path = encodeContentPath(path); return this._request('GET', `/repos/${this.__fullname}/contents/${path}`, { ref, }, cb, raw); @@ -706,11 +706,12 @@ class Repository extends Requestable { * Delete a file from a branch * @see https://developer.github.com/v3/repos/contents/#delete-a-file * @param {string} branch - the branch to delete from, or the default branch if not specified - * @param {string} path - the path of the file to remove + * @param {string} path - the unencoded path of the file to remove * @param {Requestable.callback} cb - will receive the commit in which the delete occurred * @return {Promise} - the promise for the http request */ deleteFile(branch, path, cb) { + const filePath = encodeContentPath(path); return this.getSha(branch, path) .then((response) => { const deleteCommit = { @@ -718,7 +719,7 @@ class Repository extends Requestable { sha: response.data.sha, branch, }; - return this._request('DELETE', `/repos/${this.__fullname}/contents/${path}`, deleteCommit, cb); + return this._request('DELETE', `/repos/${this.__fullname}/contents/${filePath}`, deleteCommit, cb); }); } @@ -755,7 +756,7 @@ class Repository extends Requestable { * Write a file to the repository * @see https://developer.github.com/v3/repos/contents/#update-a-file * @param {string} branch - the name of the branch - * @param {string} path - the path for the file + * @param {string} path - the unencoded path for the file * @param {string} content - the contents of the file * @param {string} message - the commit message * @param {Object} [options] - commit options @@ -771,7 +772,7 @@ class Repository extends Requestable { cb = options; options = {}; } - let filePath = path ? encodeURI(path) : ''; + let filePath = encodeContentPath(path); let shouldEncode = options.encode !== false; let commit = { branch, @@ -781,7 +782,7 @@ class Repository extends Requestable { content: shouldEncode ? Base64.encode(content) : content, }; - return this.getSha(branch, filePath) + return this.getSha(branch, path) .then((response) => { commit.sha = response.data.sha; return this._request('PUT', `/repos/${this.__fullname}/contents/${filePath}`, commit, cb); @@ -914,3 +915,12 @@ class Repository extends Requestable { } module.exports = Repository; + +/** + * Encode each component of a Contents API path while preserving directory separators. + * @param {string} path - an unencoded repository path + * @return {string} the path suitable for a Contents API URL + */ +function encodeContentPath(path) { + return path ? path.split('/').map(encodeURIComponent).join('/') : ''; +} diff --git a/test/error.spec.js b/test/error.spec.js index 9cfe5438..80bb0662 100644 --- a/test/error.spec.js +++ b/test/error.spec.js @@ -81,3 +81,61 @@ describe('Rate limit OK', function() { nock.cleanAll(); }); }); + +describe('content path encoding', function() { + let github; + let repository; + + beforeEach(function() { + github = new Github(); + repository = github.getRepo('octo', 'release-notes'); + }); + + afterEach(function() { + nock.cleanAll(); + }); + + it('should encode reserved characters when getting contents', function() { + const scope = nock('https://api.github.com:443') + .get('/repos/octo/release-notes/contents/docs/release%23notes.md') + .query({ref: 'release-2026'}) + .reply(200, {sha: 'content-sha'}); + return repository.getContents('release-2026', 'docs/release#notes.md').then(function() { + scope.done(); + }); + }); + + it('should encode reserved characters when getting a SHA', function() { + const scope = nock('https://api.github.com:443') + .get('/repos/octo/release-notes/contents/reports/draft%3F.txt') + .query({ref: 'release-2026'}) + .reply(200, {sha: 'content-sha'}); + return repository.getSha('release-2026', 'reports/draft?.txt').then(function() { + scope.done(); + }); + }); + + it('should encode reserved characters when writing a file', function() { + const scope = nock('https://api.github.com:443') + .get('/repos/octo/release-notes/contents/reports/draft%3F.txt') + .query({ref: 'release-2026'}) + .reply(200, {sha: 'content-sha'}) + .put('/repos/octo/release-notes/contents/reports/draft%3F.txt') + .reply(200, {commit: {sha: 'updated'}}); + return repository.writeFile('release-2026', 'reports/draft?.txt', 'Draft', 'Update draft').then(function() { + scope.done(); + }); + }); + + it('should encode reserved characters when deleting a file', function() { + const scope = nock('https://api.github.com:443') + .get('/repos/octo/release-notes/contents/docs/release%23notes.md') + .query({ref: 'release-2026'}) + .reply(200, {sha: 'content-sha'}) + .delete('/repos/octo/release-notes/contents/docs/release%23notes.md') + .reply(200, {commit: {sha: 'deleted'}}); + return repository.deleteFile('release-2026', 'docs/release#notes.md').then(function() { + scope.done(); + }); + }); +});