encode relative path when update url for downloaded resource (#294)

Author: ikeyan

lib/resource-handler/index.js

@@ -61,7 +61,10 @@ class ResourceHandler {
 
 					let relativePath = utils.getRelativePath(parentResource.getFilename(), respondedResource.getFilename());
 					if (self.options.prettifyUrls) {
-						relativePath = relativePath.replace(self.options.defaultFilename, '');
+						if (relativePath === self.options.defaultFilename
+							|| relativePath.endsWith('/' + self.options.defaultFilename)) {
+							relativePath = relativePath.slice(0, -self.options.defaultFilename.length);
+						}
 					}
 					const hash = utils.getHashFromUrl(childPath);
 

lib/utils/index.js

@@ -35,7 +35,11 @@ function getUnixPath (filepath) {
 function getRelativePath (path1, path2) {
 	const dirname = path.dirname(path1);
 	const relativePath = path.relative(dirname, path2);
-	return getUnixPath(relativePath);
+	const escaped = relativePath
+		.split(path.sep)
+		.map(pathComponent => encodeURIComponent(pathComponent).replace(/['()]/g, c => '%' + c.charCodeAt(0).toString(16)))
+		.join(path.sep);
+	return getUnixPath(escaped);
 }
 
 /**

test/unit/resource-handler/index.test.js

@@ -4,6 +4,7 @@ const should = require('should');
 const sinon = require('sinon');
 const Promise = require('bluebird');
 const proxyquire = require('proxyquire');
+const path = require('path');
 const Resource = require('../../../lib/resource');
 const ResourceHandler = require('../../../lib/resource-handler');
 
@@ -182,19 +183,21 @@ describe('ResourceHandler', function() {
 			pathContainer.getPaths.returns([
 				'http://first.com/img/a.jpg',
 				'http://first.com/b.jpg',
-				'http://second.com/img/c.jpg'
+				'http://second.com/img/c.jpg',
+				'http://second.com/d',
 			]);
 
-			scraperContext.requestResource.onFirstCall().returns(Promise.resolve(new Resource('http://first.com/img/a.jpg', 'local/a.jpg')));
-			scraperContext.requestResource.onSecondCall().returns(Promise.resolve(new Resource('http://first.com/b.jpg', 'local/b.jpg')));
-			scraperContext.requestResource.onThirdCall().returns(Promise.resolve(new Resource('http://second.com/img/c.jpg', 'local/c.jpg')));
+			scraperContext.requestResource.onCall(0).returns(Promise.resolve(new Resource('http://first.com/img/a.jpg', 'local' + path.sep + 'a.jpg')));
+			scraperContext.requestResource.onCall(1).returns(Promise.resolve(new Resource('http://first.com/b.jpg', 'local' + path.sep + 'b.jpg')));
+			scraperContext.requestResource.onCall(2).returns(Promise.resolve(new Resource('http://second.com/img/c.jpg', 'local' + path.sep + 'c.jpg')));
+			scraperContext.requestResource.onCall(3).returns(Promise.resolve(new Resource('http://second.com/d', 'a%b' + path.sep + '"\'( )?p=q&\\#')));
 
 			var updateChildSpy = sinon.spy(parentResource, 'updateChild');
 
 			return resHandler.downloadChildrenResources(pathContainer, parentResource).then(function () {
 				var updateTextStub = pathContainer.updateText;
 				updateTextStub.calledOnce.should.be.eql(true);
-				updateTextStub.args[0][0].length.should.be.eql(3);
+				updateTextStub.args[0][0].length.should.be.eql(4);
 				updateTextStub.args[0][0].should.containEql({
 					oldPath: 'http://first.com/img/a.jpg',
 					newPath: 'local/a.jpg'
@@ -207,7 +210,11 @@ describe('ResourceHandler', function() {
 					oldPath: 'http://second.com/img/c.jpg',
 					newPath: 'local/c.jpg'
 				});
-				updateChildSpy.calledThrice.should.be.eql(true);
+				updateTextStub.args[0][0].should.containEql({
+					oldPath: 'http://second.com/d',
+					newPath: 'a%25b/%22%27%28%20%29%3Fp%3Dq%26' + (path.sep === '\\' ? '/' : '%5C') + '%23'
+				});
+				updateChildSpy.callCount.should.be.eql(4);
 			});
 		});
 

test/unit/utils/utils-test.js

@@ -117,6 +117,13 @@ describe('Utils', function () {
 			utils.getRelativePath('index.html', 'img/1.png').should.be.equal('img/1.png');
 			utils.getRelativePath('css/1.css', 'css/2.css').should.be.equal('2.css');
 		});
+		it('should escape path components with encodeURIComponent', function () {
+			utils.getRelativePath('index.html', 'a/css?family=Open+Sans:300,400,600,700&lang=en').should.be.equal('a/css%3Ffamily%3DOpen%2BSans%3A300%2C400%2C600%2C700%26lang%3Den');
+		});
+		it('should also escape [\'()]', function () {
+			utils.getRelativePath('index.html', '\'single quote for html attrs\'').should.be.equal('%27single%20quote%20for%20html%20attrs%27');
+			utils.getRelativePath('index.html', '(parenthesizes for css url)').should.be.equal('%28parenthesizes%20for%20css%20url%29');
+		});
 	});
 
 	describe('#shortenFilename', function() {