From cadb83ac5af0911e722812d934d3133735d46ced Mon Sep 17 00:00:00 2001 From: John McLear Date: Mon, 19 Jan 2015 02:51:32 +0000 Subject: [PATCH 1/7] bumpage --- src/node/db/API.js | 11 ++++- src/node/handler/ImportHandler.js | 8 ++-- src/node/utils/ImportHtml.js | 2 +- tests/backend/specs/api/pad.js | 67 +++++++++++++++++++++++++++++++ 4 files changed, 81 insertions(+), 7 deletions(-) diff --git a/src/node/db/API.js b/src/node/db/API.js index a9df2a12f..07127309d 100644 --- a/src/node/db/API.js +++ b/src/node/db/API.js @@ -410,7 +410,16 @@ exports.setHTML = function(padID, html, callback) if(ERR(err, callback)) return; // add a new changeset with the new html to the pad - importHtml.setPadHTML(pad, cleanText(html), callback); + importHtml.setPadHTML(pad, cleanText(html), function(e){ + if(e){ + callback(new customError("HTML is malformed","apierror")); + return; + }else{ + //update the clients on the pad + padMessageHandler.updatePadClients(pad, callback); + return; + } + }); //update the clients on the pad padMessageHandler.updatePadClients(pad, callback); diff --git a/src/node/handler/ImportHandler.js b/src/node/handler/ImportHandler.js index a511637cc..676986510 100644 --- a/src/node/handler/ImportHandler.js +++ b/src/node/handler/ImportHandler.js @@ -232,11 +232,9 @@ exports.doImport = function(req, res, padId) if(!directDatabaseAccess){ var fileEnding = path.extname(srcFile).toLowerCase(); if (abiword || fileEnding == ".htm" || fileEnding == ".html") { - try{ - importHtml.setPadHTML(pad, text); - }catch(e){ - apiLogger.warn("Error importing, possibly caused by malformed HTML"); - } + importHtml.setPadHTML(pad, text, function(e){ + if(e) apiLogger.warn("Error importing, possibly caused by malformed HTML"); + }); } else { pad.setText(text); } diff --git a/src/node/utils/ImportHtml.js b/src/node/utils/ImportHtml.js index 59802f9bf..652e7fccd 100644 --- a/src/node/utils/ImportHtml.js +++ b/src/node/utils/ImportHtml.js @@ -40,7 +40,7 @@ function setPadHTML(pad, html, callback) cc.collectContent(doc); }catch(e){ apiLogger.warn("HTML was not properly formed", e); - return; // We don't process the HTML because it was bad.. + return callback(e); // We don't process the HTML because it was bad.. } var result = cc.finish(); diff --git a/tests/backend/specs/api/pad.js b/tests/backend/specs/api/pad.js index 52e7c9170..012ce4238 100644 --- a/tests/backend/specs/api/pad.js +++ b/tests/backend/specs/api/pad.js @@ -12,6 +12,7 @@ var apiVersion = 1; var testPadId = makeid(); var lastEdited = ""; var text = generateLongText(); +var ULhtml = '
'; describe('Connectivity', function(){ it('errors if can not connect', function(done) { @@ -71,6 +72,9 @@ describe('Permission', function(){ -> movePad(newPadID, originalPadId) -- Should provide consistant pad data -> getText(originalPadId) -- Should be "hello world" -> getLastEdited(padID) -- Should not be 0 + -> setHTML(padID) -- Should fail on invalid HTML + -> setHTML(padID) *3 -- Should fail on invalid HTML + -> getHTML(padID) -- Should return HTML close to posted HTML */ @@ -390,6 +394,69 @@ describe('getLastEdited', function(){ }); }) + +describe('setHTML', function(){ + it('Sets the HTML of a Pad attempting to pass ugly HTML', function(done) { + var html = "
Hello HTML
"; + api.get(endPoint('setHTML')+"&padID="+testPadId+"&html="+html) + .expect(function(res){ + if(res.body.code !== 1) throw new Error("Allowing crappy HTML to be imported") + }) + .expect('Content-Type', /json/) + .expect(200, done) + }); +}) + +describe('setHTML', function(){ + it('Sets the HTML of a Pad with a bunch of weird unordered lists inserted', function(done) { + api.get(endPoint('setHTML')+"&padID=test&html="+ULhtml) + .expect(function(res){ + if(res.body.code !== 0) throw new Error("List HTML cant be imported") + }) + .expect('Content-Type', /json/) + .expect(200, done) + }); +}) + +describe('getHTML', function(){ + // will fail due to https://github.com/ether/etherpad-lite/issues/1604 + // reminder to self this is how the HTML looks + // + //
+ // + // It will look right in the browser but the export will get it horriby wrong + + // This is what the export puts out + // + //
+ // + //
+ + it('Gets the HTML of a Pad with a bunch of weird unordered lists inserted', function(done) { + api.get(endPoint('getHTML')+"&padID=test") + .expect(function(res){ + console.log(res.body.data.html); + if(res.body.data !== ULhtml) throw new Error("Imported HTML does not match served HTML") + }) + .expect('Content-Type', /json/) + .expect(200, done) + }); +}) + + /* -> movePadForce Test From 85fffbe14ce5ca6f205434ce42b98eb4de12b6d6 Mon Sep 17 00:00:00 2001 From: John McLear Date: Mon, 19 Jan 2015 02:57:10 +0000 Subject: [PATCH 2/7] more handling --- src/node/utils/ImportHtml.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/node/utils/ImportHtml.js b/src/node/utils/ImportHtml.js index 652e7fccd..33fd91c65 100644 --- a/src/node/utils/ImportHtml.js +++ b/src/node/utils/ImportHtml.js @@ -91,6 +91,7 @@ function setPadHTML(pad, html, callback) apiLogger.debug('The changeset: ' + theChangeset); pad.setText(""); pad.appendRevision(theChangeset); + callback(null); } exports.setPadHTML = setPadHTML; From 4f637befeb449fc7e401b8a2a6624d288da86633 Mon Sep 17 00:00:00 2001 From: John McLear Date: Mon, 19 Jan 2015 02:59:17 +0000 Subject: [PATCH 3/7] more fixing --- src/node/db/API.js | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/node/db/API.js b/src/node/db/API.js index 07127309d..81dedcfef 100644 --- a/src/node/db/API.js +++ b/src/node/db/API.js @@ -420,10 +420,6 @@ exports.setHTML = function(padID, html, callback) return; } }); - - //update the clients on the pad - padMessageHandler.updatePadClients(pad, callback); - }); } From 7958f3b7232cd72e848b8922ac1ca7e2c0681515 Mon Sep 17 00:00:00 2001 From: John McLear Date: Mon, 19 Jan 2015 03:02:34 +0000 Subject: [PATCH 4/7] nearly fully working --- tests/backend/specs/api/pad.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/tests/backend/specs/api/pad.js b/tests/backend/specs/api/pad.js index 012ce4238..05f6bdd1c 100644 --- a/tests/backend/specs/api/pad.js +++ b/tests/backend/specs/api/pad.js @@ -394,7 +394,6 @@ describe('getLastEdited', function(){ }); }) - describe('setHTML', function(){ it('Sets the HTML of a Pad attempting to pass ugly HTML', function(done) { var html = "
Hello HTML
"; @@ -409,7 +408,7 @@ describe('setHTML', function(){ describe('setHTML', function(){ it('Sets the HTML of a Pad with a bunch of weird unordered lists inserted', function(done) { - api.get(endPoint('setHTML')+"&padID=test&html="+ULhtml) + api.get(endPoint('setHTML')+"&padID="+testPadId+"&html="+ULhtml) .expect(function(res){ if(res.body.code !== 0) throw new Error("List HTML cant be imported") }) @@ -446,9 +445,9 @@ describe('getHTML', function(){ //
it('Gets the HTML of a Pad with a bunch of weird unordered lists inserted', function(done) { - api.get(endPoint('getHTML')+"&padID=test") + api.get(endPoint('getHTML')+"&padID="+testPadId) .expect(function(res){ - console.log(res.body.data.html); + console.log("foo", res.body.data.html); if(res.body.data !== ULhtml) throw new Error("Imported HTML does not match served HTML") }) .expect('Content-Type', /json/) From 3463b16d1aab4277c24aef11fcb93c15f30b35da Mon Sep 17 00:00:00 2001 From: John McLear Date: Mon, 19 Jan 2015 03:04:23 +0000 Subject: [PATCH 5/7] nearly there... --- tests/backend/specs/api/pad.js | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/tests/backend/specs/api/pad.js b/tests/backend/specs/api/pad.js index 05f6bdd1c..49800e6fc 100644 --- a/tests/backend/specs/api/pad.js +++ b/tests/backend/specs/api/pad.js @@ -433,16 +433,19 @@ describe('getHTML', function(){ // It will look right in the browser but the export will get it horriby wrong // This is what the export puts out + // //
    - //
  • one
    • - //
  • 2
    • + //
  • one
    • + //
  • 2
    • //
//
- //
    - // NOTE THIS IS WHAT'S MISSING - //
  • UL2
    • + //
      + //
        + //
      • UL2
        • + //
      //
    //
    + // it('Gets the HTML of a Pad with a bunch of weird unordered lists inserted', function(done) { api.get(endPoint('getHTML')+"&padID="+testPadId) From 5967e085b7bc7efe8ab9ab1be8c728763b3f876a Mon Sep 17 00:00:00 2001 From: John McLear Date: Mon, 19 Jan 2015 15:37:29 +0000 Subject: [PATCH 6/7] fix ul tests --- tests/backend/specs/api/pad.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/backend/specs/api/pad.js b/tests/backend/specs/api/pad.js index 49800e6fc..3527f95ad 100644 --- a/tests/backend/specs/api/pad.js +++ b/tests/backend/specs/api/pad.js @@ -12,7 +12,7 @@ var apiVersion = 1; var testPadId = makeid(); var lastEdited = ""; var text = generateLongText(); -var ULhtml = '
    • one
    • 2

      • UL2
    '; +var ULhtml = '
    • one
    • 2

      • UL2
    '; describe('Connectivity', function(){ it('errors if can not connect', function(done) { @@ -450,8 +450,9 @@ describe('getHTML', function(){ it('Gets the HTML of a Pad with a bunch of weird unordered lists inserted', function(done) { api.get(endPoint('getHTML')+"&padID="+testPadId) .expect(function(res){ - console.log("foo", res.body.data.html); - if(res.body.data !== ULhtml) throw new Error("Imported HTML does not match served HTML") + var ehtml = res.body.data.html.replace("
    ", "").toLowerCase(); + var uhtml = ULhtml.toLowerCase(); + if(ehtml !== uhtml) throw new Error("Imported HTML does not match served HTML") }) .expect('Content-Type', /json/) .expect(200, done) From 860c584b425b88ba504e4cbe50c2420a375987aa Mon Sep 17 00:00:00 2001 From: John McLear Date: Mon, 19 Jan 2015 15:44:16 +0000 Subject: [PATCH 7/7] remove pointless comments --- tests/backend/specs/api/pad.js | 30 +----------------------------- 1 file changed, 1 insertion(+), 29 deletions(-) diff --git a/tests/backend/specs/api/pad.js b/tests/backend/specs/api/pad.js index 3527f95ad..6010a11ce 100644 --- a/tests/backend/specs/api/pad.js +++ b/tests/backend/specs/api/pad.js @@ -399,6 +399,7 @@ describe('setHTML', function(){ var html = "
    Hello HTML
    "; api.get(endPoint('setHTML')+"&padID="+testPadId+"&html="+html) .expect(function(res){ +console.log(res.body.code); if(res.body.code !== 1) throw new Error("Allowing crappy HTML to be imported") }) .expect('Content-Type', /json/) @@ -418,35 +419,6 @@ describe('setHTML', function(){ }) describe('getHTML', function(){ - // will fail due to https://github.com/ether/etherpad-lite/issues/1604 - // reminder to self this is how the HTML looks - //
      - //
    • one
      • - //
    • 2
      • - //
    - //
    - //
      - //
        - //
      • UL2
        • - //
      - //
    - // It will look right in the browser but the export will get it horriby wrong - - // This is what the export puts out - // - //
      - //
    • one
      • - //
    • 2
      • - //
    - //
    - //
      - //
        - //
      • UL2
        • - //
      - //
    - //
    - // - it('Gets the HTML of a Pad with a bunch of weird unordered lists inserted', function(done) { api.get(endPoint('getHTML')+"&padID="+testPadId) .expect(function(res){