From 5c012e787187171fe873676f936eaa1db2754fb7 Mon Sep 17 00:00:00 2001 From: Florian Staudacher Date: Tue, 5 May 2015 18:11:36 +0200 Subject: [PATCH] limit the parameter length for the GET request to the bookmarklet to ~2000 chars --- app/assets/javascripts/bookmarklet.js | 13 ++++++++++++- spec/javascripts/bookmarklet-spec.js | 26 ++++++++++++++++++++++++++ spec/javascripts/support/jasmine.yml | 1 + 3 files changed, 39 insertions(+), 1 deletion(-) create mode 100644 spec/javascripts/bookmarklet-spec.js diff --git a/app/assets/javascripts/bookmarklet.js b/app/assets/javascripts/bookmarklet.js index 1a9ac7117..4755490e4 100644 --- a/app/assets/javascripts/bookmarklet.js +++ b/app/assets/javascripts/bookmarklet.js @@ -1,6 +1,9 @@ // @license magnet:?xt=urn:btih:0b31508aeb0634b347b8270c7bee4d411b5d4109&dn=agpl-3.0.txt AGPL-v3-or-Later var bookmarklet = function(url, width, height, opts) { + var maxLen = 1900; // max GET request length, see #3076 + var maxTitleLen = 128; // cut title after this length, if too long + // calculate popup dimensions & placement var dim = function() { var w = window, @@ -20,7 +23,15 @@ var bookmarklet = function(url, width, height, opts) { sel = w.getSelection ? w.getSelection() : d.getSelection ? d.getSelection() : d.selection.createRange().text, - notes = sel.toString(); + notes = sel.toString(), + len = maxLen - href.length; + + if( (title+notes).length > len ) { + // shorten the text to fit in a GET request + if( title.length > maxTitleLen ) title = title.substr(0, maxTitleLen) + " ..."; + if( notes.length > (len-maxTitleLen) ) notes = notes.substr(0, len-maxTitleLen) + " ..."; + } + return "url=" + encodeURIComponent(href) + "&title=" + encodeURIComponent(title) + "¬es=" + encodeURIComponent(notes); diff --git a/spec/javascripts/bookmarklet-spec.js b/spec/javascripts/bookmarklet-spec.js new file mode 100644 index 000000000..4964e975e --- /dev/null +++ b/spec/javascripts/bookmarklet-spec.js @@ -0,0 +1,26 @@ + +describe("bookmarklet", function(){ + var fakeUrl = "http://pod.example.com/bookmarklet"; + + it("opens a popup window", function(){ + spyOn(window, "open").and.returnValue(true); + bookmarklet(fakeUrl, 800, 600); + jasmine.clock().tick(1); + expect(window.open).toHaveBeenCalled(); + }); + + it("shortens the GET string to less than 2000 characters", function(){ + var url, + selTxt = new Array(1000).join("abcdefghijklmnopqrstuvwxyz1234567890"); + + spyOn(window, "open").and.callFake(function(_url){ + url = _url; + return true; + }); + spyOn(window, "getSelection").and.returnValue(selTxt); + + bookmarklet(fakeUrl, 800, 600); + jasmine.clock().tick(1); + expect(url.length).toBeLessThan(2000); + }); +}); diff --git a/spec/javascripts/support/jasmine.yml b/spec/javascripts/support/jasmine.yml index 50f11c159..c4e49f511 100644 --- a/spec/javascripts/support/jasmine.yml +++ b/spec/javascripts/support/jasmine.yml @@ -13,6 +13,7 @@ src_files: # Precompile all scripts together for the test environment - assets/jasmine-load-all.js - assets/jasmine-jquery.js + - assets/bookmarklet.js # stylesheets #