From abc89b2015ceb60d8e20367cbf100d0a8e843ab4 Mon Sep 17 00:00:00 2001 From: Jonathan Rainville Date: Fri, 20 Apr 2018 09:52:13 -0400 Subject: [PATCH 1/4] add code to parse all files to check for http imports --- lib/core/config.js | 47 +--------- lib/core/file.js | 49 ++++++++-- lib/utils/utils.js | 49 +++++++++- test/config.js | 88 ------------------ test/contracts/contract_with_http_import.sol | 19 ++++ test/file.js | 36 +++++++- test/test.js | 94 ++++++++++++++++++++ 7 files changed, 238 insertions(+), 144 deletions(-) create mode 100644 test/contracts/contract_with_http_import.sol create mode 100644 test/test.js diff --git a/lib/core/config.js b/lib/core/config.js index f83c18279..4bb29c1e3 100644 --- a/lib/core/config.js +++ b/lib/core/config.js @@ -143,49 +143,6 @@ Config.prototype.loadContractsConfigFile = function() { this.contractsConfig = this._mergeConfig(configFilePath, configObject, this.env); }; -Config.prototype.getExternalContractUrl = function (contract) { - let url; - const RAW_URL = 'https://raw.githubusercontent.com/'; - const MALFORMED_ERROR = 'Malformed Github URL for '; - if (contract.file.startsWith('https://github')) { - const match = contract.file.match(/https:\/\/github\.[a-z]+\/(.*)/); - if (!match) { - this.logger.error(MALFORMED_ERROR + contract.file); - return null; - } - url = `${RAW_URL}${match[1].replace('blob/', '')}`; - } else if (contract.file.startsWith('git')) { - // Match values - // [0] entire input - // [1] git:// - // [2] user - // [3] repository - // [4] path - // [5] branch - const match = contract.file.match( - /(git:\/\/)?github\.[a-z]+\/([a-zA-Z0-9_\-.]+)\/([a-zA-Z0-9_\-]+)\/([a-zA-Z0-9_\-\/.]+)#?([a-zA-Z0-1_\-.]*)?/ - ); - if (!match) { - this.logger.error(MALFORMED_ERROR + contract.file); - return null; - } - let branch = match[5]; - if (!branch) { - branch = 'master'; - } - url = `${RAW_URL}${match[2]}/${match[3]}/${branch}/${match[4]}`; - } else { - url = contract.file; - } - const match = url.match( - /\.[a-z]+\/([a-zA-Z0-9_\-\/.]+)/ - ); - return { - url, - filePath: match[1] - }; -}; - Config.prototype.loadExternalContractsFiles = function() { let contracts = this.contractsConfig.contracts; for (let contractName in contracts) { @@ -194,11 +151,11 @@ Config.prototype.loadExternalContractsFiles = function() { continue; } if (contract.file.startsWith('http') || contract.file.startsWith('git')) { - const fileObj = this.getExternalContractUrl(contract); + const fileObj = utils.getExternalContractUrl(contract.file); if (!fileObj) { return this.logger.error("HTTP contract file not found: " + contract.file); } - const localFile = constants.httpContractsDirectory + fileObj.filePath; + const localFile = fileObj.filePath; this.contractsFiles.push(new File({filename: localFile, type: File.types.http, basedir: '', path: fileObj.url})); } else if (fs.existsSync(contract.file)) { this.contractsFiles.push(new File({filename: contract.file, type: File.types.dapp_file, basedir: '', path: contract.file})); diff --git a/lib/core/file.js b/lib/core/file.js index c2a65dd98..d81a90897 100644 --- a/lib/core/file.js +++ b/lib/core/file.js @@ -2,6 +2,7 @@ const async = require('async'); const fs = require('./fs.js'); const path = require('path'); const request = require('request'); +const utils = require('../utils/utils'); class File { @@ -13,21 +14,34 @@ class File { this.resolver = options.resolver; } - parseFileForImport(content, callback) { + parseFileForImport(content, isHttpContract, callback) { + if (typeof isHttpContract === 'function') { + callback = isHttpContract; + isHttpContract = false; + } const self = this; if (self.filename.indexOf('.sol') < 0) { // Only supported in Solidity return callback(); } - const regex = /import "([a-zA-Z0-9_\-.\\\/]+)";/g; + const regex = /import "([a-zA-Z0-9_\-.\\\/:]+)";/g; let matches; const filesToDownload = []; const pathWithoutFile = path.dirname(self.path); while ((matches = regex.exec(content))) { - filesToDownload.push({ + const httpFileObj = utils.getExternalContractUrl(matches[1]); + const fileObj = { fileRelativePath: path.join(path.dirname(self.filename), matches[1]), url: `${pathWithoutFile}/${matches[1]}` - }); + }; + if (httpFileObj) { + fileObj.fileRelativePath = httpFileObj.filePath; + fileObj.url = httpFileObj.url; + } else if (!isHttpContract) { + // Just a normal import + continue; + } + filesToDownload.push(fileObj); } async.each(filesToDownload, ((fileObj, eachCb) => { @@ -63,7 +77,7 @@ class File { fs.readFile(filename, next); }, function parseForImports(content, next) { - self.parseFileForImport(content, (err) => { + self.parseFileForImport(content, true, (err) => { next(err, content); }); } @@ -77,17 +91,27 @@ class File { } content (callback) { + let content; if (this.type === File.types.embark_internal) { - return callback(fs.readFileSync(fs.embarkPath(this.path)).toString()); + content = fs.readFileSync(fs.embarkPath(this.path)).toString(); } else if (this.type === File.types.dapp_file) { - return callback(fs.readFileSync(this.path).toString()); + content = fs.readFileSync(this.path).toString(); } else if (this.type === File.types.custom) { - return this.resolver(callback); + return this.resolver((theContent) => { + if (!this.parsedImports) { + this.parsedImports = true; + return this.parseFileForImport(content, () => { + callback(theContent); + }); + } + callback(theContent); + }); } else if (this.type === File.types.http) { - this.downloadFile(this.filename, this.path, (content) => { + return this.downloadFile(this.filename, this.path, (content) => { if (!content) { return callback(content); } + this.parsedImports = true; this.path = this.filename; this.type = File.types.dapp_file; callback(content); @@ -95,6 +119,13 @@ class File { } else { throw new Error("unknown file: " + this.filename); } + if (!this.parsedImports) { + this.parsedImports = true; + return this.parseFileForImport(content, () => { + callback(content); + }); + } + callback(content); } } diff --git a/lib/utils/utils.js b/lib/utils/utils.js index c38c96eb5..2d85d9604 100644 --- a/lib/utils/utils.js +++ b/lib/utils/utils.js @@ -6,6 +6,7 @@ let https = require('follow-redirects').https; let shelljs = require('shelljs'); var tar = require('tar'); var propose = require('propose'); +const constants = require('../constants'); //let fs = require('../core/fs.js'); let o_fs = require('fs-extra'); @@ -127,6 +128,51 @@ function pwd() { return process.env.PWD || process.cwd(); } +function getExternalContractUrl(file) { + let url; + const RAW_URL = 'https://raw.githubusercontent.com/'; + const MALFORMED_ERROR = 'Malformed Github URL for '; + if (file.startsWith('https://github')) { + const match = file.match(/https:\/\/github\.[a-z]+\/(.*)/); + if (!match) { + console.error(MALFORMED_ERROR + file); + return null; + } + url = `${RAW_URL}${match[1].replace('blob/', '')}`; + } else if (file.startsWith('git')) { + // Match values + // [0] entire input + // [1] git:// + // [2] user + // [3] repository + // [4] path + // [5] branch + const match = file.match( + /(git:\/\/)?github\.[a-z]+\/([a-zA-Z0-9_\-.]+)\/([a-zA-Z0-9_\-]+)\/([a-zA-Z0-9_\-\/.]+)#?([a-zA-Z0-1_\-.]*)?/ + ); + if (!match) { + console.error(MALFORMED_ERROR + file); + return null; + } + let branch = match[5]; + if (!branch) { + branch = 'master'; + } + url = `${RAW_URL}${match[2]}/${match[3]}/${branch}/${match[4]}`; + } else if (file.startsWith('http')) { + url = file; + } else { + return null; + } + const match = url.match( + /\.[a-z]+\/([a-zA-Z0-9_\-\/.]+)/ + ); + return { + url, + filePath: constants.httpContractsDirectory + match[1] + }; +} + module.exports = { joinPath: joinPath, filesMatchingPattern: filesMatchingPattern, @@ -143,5 +189,6 @@ module.exports = { downloadFile: downloadFile, extractTar: extractTar, proposeAlternative: proposeAlternative, - pwd: pwd + pwd: pwd, + getExternalContractUrl }; diff --git a/test/config.js b/test/config.js index 0f8af6147..ebc2b594d 100644 --- a/test/config.js +++ b/test/config.js @@ -57,94 +57,6 @@ describe('embark.Config', function () { }); }); - describe('#getExternalContractUrl', function () { - it('should get the right url for a https://github file', function () { - const fileObj = config.getExternalContractUrl( - {file: 'https://github.com/embark-framework/embark/blob/master/test_app/app/contracts/simple_storage.sol'} - ); - assert.deepEqual(fileObj, - { - filePath: 'embark-framework/embark/master/test_app/app/contracts/simple_storage.sol', - url: 'https://raw.githubusercontent.com/embark-framework/embark/master/test_app/app/contracts/simple_storage.sol' - }); - }); - - it('should fail for a malformed https://github file', function () { - const fileObj = config.getExternalContractUrl( - {file: 'https://github/embark-framework/embark/blob/master/test_app/app/contracts/simple_storage.sol'} - ); - assert.strictEqual(fileObj, null); - }); - - it('should get the right url for a git:// file with no branch #', function () { - const fileObj = config.getExternalContractUrl( - {file: 'git://github.com/status-im/contracts/contracts/identity/ERC725.sol'} - ); - assert.deepEqual(fileObj, - { - filePath: 'status-im/contracts/master/contracts/identity/ERC725.sol', - url: 'https://raw.githubusercontent.com/status-im/contracts/master/contracts/identity/ERC725.sol' - }); - }); - - it('should get the right url for a git:// file with a branch #', function () { - const fileObj = config.getExternalContractUrl( - {file: 'git://github.com/status-im/contracts/contracts/identity/ERC725.sol#myBranch'} - ); - assert.deepEqual(fileObj, - { - filePath: 'status-im/contracts/myBranch/contracts/identity/ERC725.sol', - url: 'https://raw.githubusercontent.com/status-im/contracts/myBranch/contracts/identity/ERC725.sol' - }); - }); - - it('should fail when the git:// file is malformed', function () { - const fileObj = config.getExternalContractUrl( - {file: 'git://github.com/identity/ERC725.sol#myBranch'} - ); - assert.strictEqual(fileObj, null); - }); - - it('should get the right url with a github.com file without branch #', function () { - const fileObj = config.getExternalContractUrl( - {file: 'github.com/status-im/contracts/contracts/identity/ERC725.sol'} - ); - assert.deepEqual(fileObj, - { - filePath: 'status-im/contracts/master/contracts/identity/ERC725.sol', - url: 'https://raw.githubusercontent.com/status-im/contracts/master/contracts/identity/ERC725.sol' - }); - }); - - it('should get the right url with a github.com file with branch #', function () { - const fileObj = config.getExternalContractUrl( - {file: 'github.com/status-im/contracts/contracts/identity/ERC725.sol#theBranch'} - ); - assert.deepEqual(fileObj, - { - filePath: 'status-im/contracts/theBranch/contracts/identity/ERC725.sol', - url: 'https://raw.githubusercontent.com/status-im/contracts/theBranch/contracts/identity/ERC725.sol' - }); - }); - - it('should fail with a malformed github.com url', function () { - const fileObj = config.getExternalContractUrl( - {file: 'github/status-im/contracts/contracts/identity/ERC725.sol#theBranch'} - ); - assert.strictEqual(fileObj, null); - }); - - it('should succeed with a generic http url', function () { - const fileObj = config.getExternalContractUrl( - {file: 'http://myurl.com/myFile.sol'} - ); - assert.deepEqual(fileObj, { - filePath: 'myFile.sol', - url: 'http://myurl.com/myFile.sol' - }); - }); - }); - describe('#loadExternalContractsFiles', function () { it('should create the right list of files and download', function () { config.contractsFiles = []; diff --git a/test/contracts/contract_with_http_import.sol b/test/contracts/contract_with_http_import.sol new file mode 100644 index 000000000..b0dc7b8cf --- /dev/null +++ b/test/contracts/contract_with_http_import.sol @@ -0,0 +1,19 @@ +pragma solidity ^0.4.7; +contract SimpleStorage { + uint public storedData; + import "https://github.com/embark-framework/embark/blob/develop/test_apps/contracts_app/contracts/contract_args.sol"; + + function SimpleStorage(uint initialValue) { + storedData = initialValue; + } + + function set(uint x) { + storedData = x; + } + + function get() constant returns (uint retVal) { + return storedData; + } + +} + diff --git a/test/file.js b/test/file.js index 1fb474054..db913710c 100644 --- a/test/file.js +++ b/test/file.js @@ -16,7 +16,7 @@ describe('embark.File', function () { cb(); }); - file.parseFileForImport(contract, () => { + file.parseFileForImport(contract, true, () => { assert.strictEqual(downloadFileStub.callCount, 1); assert.strictEqual(downloadFileStub.firstCall.args[0], path.normalize('.embark/contracts/embark-framework/embark/master/test_app/app/contracts/ownable.sol')); @@ -25,5 +25,39 @@ describe('embark.File', function () { done(); }); }); + + it('should find all the imports but not call download because not a http contract', function (done) { + const contract = fs.readFileSync('./test/contracts/contract_with_import.sol').toString(); + const file = new File({filename: '.embark/contracts/embark-framework/embark/master/test_app/app/contracts/simple_storage.sol', + path: 'https://raw.githubusercontent.com/embark-framework/embark/develop/test_apps/test_app/app/contracts/simple_storage.sol'}); + const downloadFileStub = sinon.stub(file, 'downloadFile') + .callsFake((path, url, cb) => { + cb(); + }); + + file.parseFileForImport(contract, () => { + assert.strictEqual(downloadFileStub.callCount, 0); + done(); + }); + }); + + it('should find all the imports and call downlaod because it is an http import', function (done) { + const contract = fs.readFileSync('./test/contracts/contract_with_http_import.sol').toString(); + const file = new File({filename: '.embark/contracts/embark-framework/embark/master/test_app/app/contracts/simple_storage.sol', + path: 'https://raw.githubusercontent.com/embark-framework/embark/develop/test_apps/test_app/app/contracts/simple_storage.sol'}); + const downloadFileStub = sinon.stub(file, 'downloadFile') + .callsFake((path, url, cb) => { + cb(); + }); + + file.parseFileForImport(contract, () => { + assert.strictEqual(downloadFileStub.callCount, 1); + assert.strictEqual(downloadFileStub.firstCall.args[0], + '.embark/contracts/embark-framework/embark/develop/test_apps/contracts_app/contracts/contract_args.sol'); + assert.strictEqual(downloadFileStub.firstCall.args[1], + 'https://raw.githubusercontent.com/embark-framework/embark/develop/test_apps/contracts_app/contracts/contract_args.sol'); + done(); + }); + }); }); }); diff --git a/test/test.js b/test/test.js new file mode 100644 index 000000000..e2d0ead77 --- /dev/null +++ b/test/test.js @@ -0,0 +1,94 @@ +/*global describe, it*/ +const Utils = require('../lib/utils/utils'); +const assert = require('assert'); +const constants = require('../lib/constants'); + +describe('embark.utils', function () { + describe('#getExternalContractUrl', function () { + it('should get the right url for a https://github file', function () { + const fileObj = Utils.getExternalContractUrl( + 'https://github.com/embark-framework/embark/blob/master/test_app/app/contracts/simple_storage.sol' + ); + assert.deepEqual(fileObj, + { + filePath: constants.httpContractsDirectory + 'embark-framework/embark/master/test_app/app/contracts/simple_storage.sol', + url: 'https://raw.githubusercontent.com/embark-framework/embark/master/test_app/app/contracts/simple_storage.sol' + }); + }); + + it('should fail for a malformed https://github file', function () { + const fileObj = Utils.getExternalContractUrl( + 'https://github/embark-framework/embark/blob/master/test_app/app/contracts/simple_storage.sol' + ); + assert.strictEqual(fileObj, null); + }); + + it('should get the right url for a git:// file with no branch #', function () { + const fileObj = Utils.getExternalContractUrl( + 'git://github.com/status-im/contracts/contracts/identity/ERC725.sol' + ); + assert.deepEqual(fileObj, + { + filePath: constants.httpContractsDirectory + 'status-im/contracts/master/contracts/identity/ERC725.sol', + url: 'https://raw.githubusercontent.com/status-im/contracts/master/contracts/identity/ERC725.sol' + }); + }); + + it('should get the right url for a git:// file with a branch #', function () { + const fileObj = Utils.getExternalContractUrl( + 'git://github.com/status-im/contracts/contracts/identity/ERC725.sol#myBranch' + ); + assert.deepEqual(fileObj, + { + filePath: constants.httpContractsDirectory + 'status-im/contracts/myBranch/contracts/identity/ERC725.sol', + url: 'https://raw.githubusercontent.com/status-im/contracts/myBranch/contracts/identity/ERC725.sol' + }); + }); + + it('should fail when the git:// file is malformed', function () { + const fileObj = Utils.getExternalContractUrl( + 'git://github.com/identity/ERC725.sol#myBranch' + ); + assert.strictEqual(fileObj, null); + }); + + it('should get the right url with a github.com file without branch #', function () { + const fileObj = Utils.getExternalContractUrl( + 'github.com/status-im/contracts/contracts/identity/ERC725.sol' + ); + assert.deepEqual(fileObj, + { + filePath: constants.httpContractsDirectory + 'status-im/contracts/master/contracts/identity/ERC725.sol', + url: 'https://raw.githubusercontent.com/status-im/contracts/master/contracts/identity/ERC725.sol' + }); + }); + + it('should get the right url with a github.com file with branch #', function () { + const fileObj = Utils.getExternalContractUrl( + 'github.com/status-im/contracts/contracts/identity/ERC725.sol#theBranch' + ); + assert.deepEqual(fileObj, + { + filePath: constants.httpContractsDirectory + 'status-im/contracts/theBranch/contracts/identity/ERC725.sol', + url: 'https://raw.githubusercontent.com/status-im/contracts/theBranch/contracts/identity/ERC725.sol' + }); + }); + + it('should fail with a malformed github.com url', function () { + const fileObj = Utils.getExternalContractUrl( + 'github/status-im/contracts/contracts/identity/ERC725.sol#theBranch' + ); + assert.strictEqual(fileObj, null); + }); + + it('should succeed with a generic http url', function () { + const fileObj = Utils.getExternalContractUrl( + 'http://myurl.com/myFile.sol' + ); + assert.deepEqual(fileObj, { + filePath: constants.httpContractsDirectory + 'myFile.sol', + url: 'http://myurl.com/myFile.sol' + }); + }); + }); +}); From 9bf06aebce961603c64794d6d929a4a74a28ca2e Mon Sep 17 00:00:00 2001 From: Jonathan Rainville Date: Fri, 20 Apr 2018 10:03:03 -0400 Subject: [PATCH 2/4] refacotr how we handle files already parsed --- lib/core/file.js | 28 ++++++++++++---------------- test/file.js | 22 ++++++++++++++++++++++ 2 files changed, 34 insertions(+), 16 deletions(-) diff --git a/lib/core/file.js b/lib/core/file.js index d81a90897..d799b50db 100644 --- a/lib/core/file.js +++ b/lib/core/file.js @@ -15,11 +15,16 @@ class File { } parseFileForImport(content, isHttpContract, callback) { + const self = this; if (typeof isHttpContract === 'function') { callback = isHttpContract; isHttpContract = false; } - const self = this; + if (self.parsedImports) { + // We already parsed this file + return callback(); + } + self.parsedImports = true; if (self.filename.indexOf('.sol') < 0) { // Only supported in Solidity return callback(); @@ -98,20 +103,15 @@ class File { content = fs.readFileSync(this.path).toString(); } else if (this.type === File.types.custom) { return this.resolver((theContent) => { - if (!this.parsedImports) { - this.parsedImports = true; - return this.parseFileForImport(content, () => { - callback(theContent); - }); - } - callback(theContent); + this.parseFileForImport(content, () => { + callback(theContent); + }); }); } else if (this.type === File.types.http) { return this.downloadFile(this.filename, this.path, (content) => { if (!content) { return callback(content); } - this.parsedImports = true; this.path = this.filename; this.type = File.types.dapp_file; callback(content); @@ -119,13 +119,9 @@ class File { } else { throw new Error("unknown file: " + this.filename); } - if (!this.parsedImports) { - this.parsedImports = true; - return this.parseFileForImport(content, () => { - callback(content); - }); - } - callback(content); + return this.parseFileForImport(content, () => { + callback(content); + }); } } diff --git a/test/file.js b/test/file.js index db913710c..bd261c45f 100644 --- a/test/file.js +++ b/test/file.js @@ -59,5 +59,27 @@ describe('embark.File', function () { done(); }); }); + + it('should find all the imports but only once if called twice', function (done) { + const contract = fs.readFileSync('./test/contracts/contract_with_http_import.sol').toString(); + const file = new File({filename: '.embark/contracts/embark-framework/embark/master/test_app/app/contracts/simple_storage.sol', + path: 'https://raw.githubusercontent.com/embark-framework/embark/develop/test_apps/test_app/app/contracts/simple_storage.sol'}); + const downloadFileStub = sinon.stub(file, 'downloadFile') + .callsFake((path, url, cb) => { + cb(); + }); + + file.parseFileForImport(contract, () => { + // Parse again + file.parseFileForImport(contract, () => { + assert.strictEqual(downloadFileStub.callCount, 1); + assert.strictEqual(downloadFileStub.firstCall.args[0], + '.embark/contracts/embark-framework/embark/develop/test_apps/contracts_app/contracts/contract_args.sol'); + assert.strictEqual(downloadFileStub.firstCall.args[1], + 'https://raw.githubusercontent.com/embark-framework/embark/develop/test_apps/contracts_app/contracts/contract_args.sol'); + done(); + }); + }); + }); }); }); From c1bed28c004dae68fba7195ec383e19a8f51c5a9 Mon Sep 17 00:00:00 2001 From: Jonathan Rainville Date: Fri, 20 Apr 2018 11:39:17 -0400 Subject: [PATCH 3/4] code and test importing the http contract --- lib/core/file.js | 29 +++++++++------- lib/modules/solidity/solcP.js | 5 +++ test/contracts/contract_with_http_import.sol | 2 +- test/contracts/contract_with_import.sol | 2 +- .../contracts/SimpleStorageWithHttpImport.sol | 34 +++++++++++++++++++ test_apps/test_app/config/contracts.json | 6 ++++ test_apps/test_app/test/http_contract_test.js | 16 +++++++-- 7 files changed, 77 insertions(+), 17 deletions(-) create mode 100644 test_apps/test_app/app/contracts/SimpleStorageWithHttpImport.sol diff --git a/lib/core/file.js b/lib/core/file.js index d799b50db..ec438ed71 100644 --- a/lib/core/file.js +++ b/lib/core/file.js @@ -20,16 +20,11 @@ class File { callback = isHttpContract; isHttpContract = false; } - if (self.parsedImports) { - // We already parsed this file - return callback(); - } - self.parsedImports = true; if (self.filename.indexOf('.sol') < 0) { // Only supported in Solidity - return callback(); + return callback(null, content); } - const regex = /import "([a-zA-Z0-9_\-.\\\/:]+)";/g; + const regex = /import ["|']([a-zA-Z0-9_\-.\\\/:]+)";/g; let matches; const filesToDownload = []; const pathWithoutFile = path.dirname(self.path); @@ -40,6 +35,9 @@ class File { url: `${pathWithoutFile}/${matches[1]}` }; if (httpFileObj) { + // Replace http import by filePath import in content + content = content.replace(matches[1], httpFileObj.filePath); + fileObj.fileRelativePath = httpFileObj.filePath; fileObj.url = httpFileObj.url; } else if (!isHttpContract) { @@ -49,11 +47,18 @@ class File { filesToDownload.push(fileObj); } + if (self.downloadedImports) { + // We already parsed this file + return callback(null, content); + } + self.downloadedImports = true; async.each(filesToDownload, ((fileObj, eachCb) => { self.downloadFile(fileObj.fileRelativePath, fileObj.url, (_content) => { eachCb(); }); - }), callback); + }), (err) => { + callback(err, content); + }); } downloadFile (filename, url, callback) { @@ -103,8 +108,8 @@ class File { content = fs.readFileSync(this.path).toString(); } else if (this.type === File.types.custom) { return this.resolver((theContent) => { - this.parseFileForImport(content, () => { - callback(theContent); + this.parseFileForImport(theContent, (err, newContent) => { + callback(newContent); }); }); } else if (this.type === File.types.http) { @@ -119,8 +124,8 @@ class File { } else { throw new Error("unknown file: " + this.filename); } - return this.parseFileForImport(content, () => { - callback(content); + return this.parseFileForImport(content, (err, newContent) => { + callback(newContent); }); } diff --git a/lib/modules/solidity/solcP.js b/lib/modules/solidity/solcP.js index 8042ed839..2beb3305e 100644 --- a/lib/modules/solidity/solcP.js +++ b/lib/modules/solidity/solcP.js @@ -3,8 +3,13 @@ let solc; const fs = require('fs-extra'); const path = require('path'); const constants = require('../../constants'); +const Utils = require('../../utils/utils'); function findImports(filename) { + if (filename.startsWith('http') || filename.startsWith('git')) { + const fileObj = Utils.getExternalContractUrl(filename); + filename = fileObj.filePath; + } if (fs.existsSync(filename)) { return {contents: fs.readFileSync(filename).toString()}; } diff --git a/test/contracts/contract_with_http_import.sol b/test/contracts/contract_with_http_import.sol index b0dc7b8cf..29b08cda9 100644 --- a/test/contracts/contract_with_http_import.sol +++ b/test/contracts/contract_with_http_import.sol @@ -1,7 +1,7 @@ pragma solidity ^0.4.7; +import "https://github.com/embark-framework/embark/blob/develop/test_apps/contracts_app/contracts/contract_args.sol"; contract SimpleStorage { uint public storedData; - import "https://github.com/embark-framework/embark/blob/develop/test_apps/contracts_app/contracts/contract_args.sol"; function SimpleStorage(uint initialValue) { storedData = initialValue; diff --git a/test/contracts/contract_with_import.sol b/test/contracts/contract_with_import.sol index 7a07a3040..0f206a406 100644 --- a/test/contracts/contract_with_import.sol +++ b/test/contracts/contract_with_import.sol @@ -1,7 +1,7 @@ pragma solidity ^0.4.7; +import "./ownable.sol"; contract SimpleStorage { uint public storedData; - import "./ownable.sol"; function SimpleStorage(uint initialValue) { storedData = initialValue; diff --git a/test_apps/test_app/app/contracts/SimpleStorageWithHttpImport.sol b/test_apps/test_app/app/contracts/SimpleStorageWithHttpImport.sol new file mode 100644 index 000000000..21c3505b5 --- /dev/null +++ b/test_apps/test_app/app/contracts/SimpleStorageWithHttpImport.sol @@ -0,0 +1,34 @@ +pragma solidity ^0.4.17; + +import "https://github.com/embark-framework/embark/blob/develop/test_apps/contracts_app/contracts/ownable.sol"; + + +contract SimpleStorageWithHttpImport is Ownable { + uint public storedData; + + function() public payable { } + + function SimpleStorageWithHttpImport(uint initialValue) public { + storedData = initialValue; + } + + function set(uint x) public { + storedData = x; + for(uint i = 0; i < 1000; i++) { + storedData += i; + } + } + + function set2(uint x, uint unusedGiveWarning) public onlyOwner { + storedData = x; + } + + function get() public view returns (uint retVal) { + return storedData; + } + + function getS() public pure returns (string d) { + return "hello"; + } + +} diff --git a/test_apps/test_app/config/contracts.json b/test_apps/test_app/config/contracts.json index 4f59c2768..b1f0597fa 100644 --- a/test_apps/test_app/config/contracts.json +++ b/test_apps/test_app/config/contracts.json @@ -74,6 +74,12 @@ }, "Identity": { "file": "https://github.com/status-im/contracts/blob/master/contracts/identity/Identity.sol" + }, + "SimpleStorageWithHttpImport": { + "fromIndex": 0, + "args": [ + 100 + ] } }, "afterDeploy": [ diff --git a/test_apps/test_app/test/http_contract_test.js b/test_apps/test_app/test/http_contract_test.js index c1cddd67d..98b2d34e3 100644 --- a/test_apps/test_app/test/http_contract_test.js +++ b/test_apps/test_app/test/http_contract_test.js @@ -3,10 +3,9 @@ const fs = require('fs-extra'); const assert = require('assert'); describe('http contracts', () => { - const contractPath = '.embark/contracts/status-im/contracts/master/contracts/identity/Identity.sol'; - const contractImportPath = '.embark/contracts/status-im/contracts/master/contracts/identity/ERC725.sol'; it('should have downloaded the file in .embark/contracts', (done) => { + const contractPath = '.embark/contracts/status-im/contracts/master/contracts/identity/Identity.sol'; fs.access(contractPath, (err) => { if (err) { assert.fail(contractPath + ' was not downloaded'); @@ -16,9 +15,20 @@ describe('http contracts', () => { }); it('should have downloaded the file import file too', (done) => { + const contractImportPath = '.embark/contracts/status-im/contracts/master/contracts/identity/ERC725.sol'; fs.access(contractImportPath, (err) => { if (err) { - assert.fail(contractPath + ' was not downloaded'); + assert.fail(contractImportPath + ' was not downloaded'); + } + done(); + }); + }); + + it('should have downloaded the http import in SimpleStorageWithHttpImport', (done) => { + const contractImportPath = '.embark/contracts/embark-framework/embark/develop/test_apps/contracts_app/contracts/ownable.sol'; + fs.access(contractImportPath, (err) => { + if (err) { + assert.fail(contractImportPath + ' was not downloaded'); } done(); }); From d48c9714712a1dc1b517ea895de8a546dc7be87d Mon Sep 17 00:00:00 2001 From: Jonathan Rainville Date: Fri, 20 Apr 2018 12:04:27 -0400 Subject: [PATCH 4/4] improve url regexes --- lib/core/file.js | 2 +- lib/utils/utils.js | 4 ++-- test/{test.js => utils.js} | 0 3 files changed, 3 insertions(+), 3 deletions(-) rename test/{test.js => utils.js} (100%) diff --git a/lib/core/file.js b/lib/core/file.js index ec438ed71..eca136b13 100644 --- a/lib/core/file.js +++ b/lib/core/file.js @@ -24,7 +24,7 @@ class File { // Only supported in Solidity return callback(null, content); } - const regex = /import ["|']([a-zA-Z0-9_\-.\\\/:]+)";/g; + const regex = /import ["|']([-a-zA-Z0-9@:%_+.~#?&\/=]+)["|'];/g; let matches; const filesToDownload = []; const pathWithoutFile = path.dirname(self.path); diff --git a/lib/utils/utils.js b/lib/utils/utils.js index 2d85d9604..0063c1ec3 100644 --- a/lib/utils/utils.js +++ b/lib/utils/utils.js @@ -148,7 +148,7 @@ function getExternalContractUrl(file) { // [4] path // [5] branch const match = file.match( - /(git:\/\/)?github\.[a-z]+\/([a-zA-Z0-9_\-.]+)\/([a-zA-Z0-9_\-]+)\/([a-zA-Z0-9_\-\/.]+)#?([a-zA-Z0-1_\-.]*)?/ + /(git:\/\/)?github\.[a-z]+\/([-a-zA-Z0-9@:%_+.~#?&=]+)\/([-a-zA-Z0-9@:%_+.~#?&=]+)\/([-a-zA-Z0-9@:%_+.~?\/&=]+)#?([a-zA-Z0-1\/_.-]*)?/ ); if (!match) { console.error(MALFORMED_ERROR + file); @@ -165,7 +165,7 @@ function getExternalContractUrl(file) { return null; } const match = url.match( - /\.[a-z]+\/([a-zA-Z0-9_\-\/.]+)/ + /\.[a-z]+\/([-a-zA-Z0-9@:%_+.~#?&\/=]+)/ ); return { url, diff --git a/test/test.js b/test/utils.js similarity index 100% rename from test/test.js rename to test/utils.js