From ede33370d761546f192be49e64a51c12234c2220 Mon Sep 17 00:00:00 2001 From: Ian Bicking Date: Fri, 9 Jun 2017 16:37:30 -0500 Subject: [PATCH] Pile o patches (#3010) * Revert "allow for EXTRA_CONTENT_ORIGIN" This reverts commit 1e6e9d7af65907fa53fd4be9374ddfb96ae4af4b. Fixes #2963 * Fix #2426, make sure contentOrigin isn't undefined in CSP header * Fix #2679, avoid including README and .template files in zip * Change update_manifest.py to use Py2+3 syntax * Fix 2767, make update_manifest.py resilient to a bad manifest.json Also fix the logic that keeps the version always going up * Fix #2856, remove fromMakeError from Sentry reports * Fix #2941, validate backend argument for manifest * Fix #2985, note Sentry IP collection is turned off * Fix #2967, don't give an error when document.body is missing This notices and reports the specific element that is in the page, that isn't an HTML document * Fix #2994, avoid error when closing during resize There is a timer attached to updateElementSize, so that it can run after the selector is unloaded, leading to an unnecessary error * Fix #2647, add a note about version bumps to export docs All the other information in #2647 is implicit in the release checklist --- Makefile | 2 +- addon/webextension/background/main.js | 10 +++++++ addon/webextension/background/senderror.js | 2 +- addon/webextension/selector/ui.js | 5 ++++ addon/webextension/selector/uicontrol.js | 13 ++++++--- bin/build-scripts/update_manifest.py | 32 ++++++++++++++-------- docs/METRICS.md | 2 +- docs/export-to-firefox.md | 2 +- server/src/config.js | 7 ----- server/src/server.js | 5 ++-- 10 files changed, 51 insertions(+), 29 deletions(-) diff --git a/Makefile b/Makefile index a566d1bd67..d342d4a056 100644 --- a/Makefile +++ b/Makefile @@ -101,7 +101,7 @@ flake8: $(VENV) .PHONY: zip zip: addon # FIXME: should remove web-ext-artifacts/*.zip first - ./node_modules/.bin/web-ext build --source-dir addon/webextension/ + ./node_modules/.bin/web-ext build --source-dir addon/webextension/ --ignore-files "**/README.md" --ignore-files "**/*.template" mv web-ext-artifacts/firefox_screenshots*.zip build/screenshots.zip # We'll try to remove this directory, but it's no big deal if we can't: rmdir web-ext-artifacts || true diff --git a/addon/webextension/background/main.js b/addon/webextension/background/main.js index 2387838938..98411ff2d9 100644 --- a/addon/webextension/background/main.js +++ b/addon/webextension/background/main.js @@ -276,6 +276,16 @@ this.main = (function() { }); }); + communication.register("abortNoDocumentBody", (sender, tagName) => { + tagName = String(tagName || "").replace(/[^a-z0-9]/ig, ""); + sendEvent("abort-start-shot", `document-is-${tagName}`); + // Note, we only show the error but don't report it, as we know that we can't + // take shots of these pages: + senderror.showError({ + popupMessage: "UNSHOOTABLE_PAGE" + }); + }); + // Note: this signal is only needed until bug 1357589 is fixed. communication.register("openTermsPage", () => { return catcher.watchPromise(browser.tabs.create({url: "https://www.mozilla.org/about/legal/terms/services/"})); diff --git a/addon/webextension/background/senderror.js b/addon/webextension/background/senderror.js index fed72e0a65..2f9db3222a 100644 --- a/addon/webextension/background/senderror.js +++ b/addon/webextension/background/senderror.js @@ -105,7 +105,7 @@ this.senderror = (function() { ); let rest = {}; for (let attr in e) { - if (!["name", "message", "stack", "multilineStack", "popupMessage", "version", "sentryPublicDSN", "help"].includes(attr)) { + if (!["name", "message", "stack", "multilineStack", "popupMessage", "version", "sentryPublicDSN", "help", "fromMakeError"].includes(attr)) { rest[attr] = e[attr]; } } diff --git a/addon/webextension/selector/ui.js b/addon/webextension/selector/ui.js index 05821961ad..34438dd2b8 100644 --- a/addon/webextension/selector/ui.js +++ b/addon/webextension/selector/ui.js @@ -272,6 +272,11 @@ this.ui = (function() { // eslint-disable-line no-unused-vars }, updateElementSize() { + if (!this.element) { + // This can happen if the selector is unloaded during the resize adjustment + // time-delay + return; + } this.element.style.height = window.innerHeight + "px"; this.element.style.width = window.innerWidth + "px"; }, diff --git a/addon/webextension/selector/uicontrol.js b/addon/webextension/selector/uicontrol.js index c390df706f..d48b2e5ea8 100644 --- a/addon/webextension/selector/uicontrol.js +++ b/addon/webextension/selector/uicontrol.js @@ -783,14 +783,14 @@ this.uicontrol = (function() { }; let documentWidth = Math.max( - document.body.clientWidth, + document.body && document.body.clientWidth, document.documentElement.clientWidth, - document.body.scrollWidth, + document.body && document.body.scrollWidth, document.documentElement.scrollWidth); let documentHeight = Math.max( - document.body.clientHeight, + document.body && document.body.clientHeight, document.documentElement.clientHeight, - document.body.scrollHeight, + document.body && document.body.scrollHeight, document.documentElement.scrollHeight); function scrollIfByEdge(pageX, pageY) { @@ -818,6 +818,11 @@ this.uicontrol = (function() { let shouldOnboard = typeof slides !== "undefined"; exports.activate = function() { + if (!document.body) { + callBackground("abortNoDocumentBody", document.documentElement.tagName); + selectorLoader.unloadModules(); + return; + } if (isFrameset()) { callBackground("abortFrameset"); selectorLoader.unloadModules(); diff --git a/bin/build-scripts/update_manifest.py b/bin/build-scripts/update_manifest.py index 8a2abcbf07..ec1d72c80e 100755 --- a/bin/build-scripts/update_manifest.py +++ b/bin/build-scripts/update_manifest.py @@ -13,11 +13,11 @@ import re if not sys.argv[1:] or "-h" in sys.argv or "--help" in sys.argv: - print "Usage: %s MANIFEST_TEMPLATE MANIFEST_JSON" % (os.path.basename(sys.argv[0])) - print " Writes MANIFEST_JSON based on MANIFEST_TEMPLATE" - print " Uses build/.backend.txt to figure out the backend configuration" - print " Uses package.json to determine the version (and adds a timestamp)" - print " If $SCREENSHOTS_MINOR_VERSION is set, use that instead of a timestamp for that version" + print("Usage: %s MANIFEST_TEMPLATE MANIFEST_JSON" % (os.path.basename(sys.argv[0]))) + print(" Writes MANIFEST_JSON based on MANIFEST_TEMPLATE") + print(" Uses build/.backend.txt to figure out the backend configuration") + print(" Uses package.json to determine the version (and adds a timestamp)") + print(" If $SCREENSHOTS_MINOR_VERSION is set, use that instead of a timestamp for that version") sys.exit() template = open(sys.argv[1]).read() @@ -26,8 +26,12 @@ last_version = None if os.path.exists(output_file): if output_file.endswith(".json"): - output_data = json.load(open(output_file)) - last_version = output_data["version"] + try: + output_data = json.load(open(output_file)) + except ValueError: + print("Ignoring current manifest in %s" % output_file) + else: + last_version = output_data["version"] elif output_file.endswith(".rdf"): rdf_content = open(output_file).read() match = re.search(r'(.*?)', rdf_content) @@ -43,11 +47,17 @@ package_json_version = package_json["version"].split(".") while True: version = "%s.%s.%s" % (package_json_version[0], package_json_version[1], now_timestamp) - if last_version == version: - now_timestamp += 1 - else: - break + if last_version: + last_parts = last_version.split(".") + version_parts = version.split(".") + if last_parts[0] == version_parts[0] and last_parts[1] == version_parts[1] and int(last_parts[2]) >= int(version_parts[2]): + now_timestamp += 1 + continue + break backend = open("build/.backend.txt").read().strip() +if not re.search(r'^https?://[^/]+/?$', backend): + print("Error: bad backend (must be fully qualified URL): %r" % backend) + sys.exit(1) template = template.replace("__VERSION__", version) # Some places we use the port: diff --git a/docs/METRICS.md b/docs/METRICS.md index 9cb93e2201..6aa0b1af5b 100644 --- a/docs/METRICS.md +++ b/docs/METRICS.md @@ -132,6 +132,7 @@ The primary change was in `server/src/pages/shot/share-buttons.js` 15. ~~Uninstall the add-on `addon/uninstall` (fired internally, regardless of how it is uninstalled)~~ 16. ~~Hit shot button on a page that can't be shot (XUL page) `addon/abort-start-shot/xul-page`~~ 17. [x] Hit shot button on a page that uses `` and can't be shot, `addon/abort-start-shot/frame-page` +18. [x] Hit shot button on a page (like an SVG image) that doesn't have a `document.body` and can't be shot, `addon/abort-start-shot/document-is-TAGNAME` where TAGNAME is the tag of `document.documentElement` (e.g., `document-is-svg`) 99. Toggle shot button off `addon/cancel-shot/toolbar-button` 99. Bad response when trying to login `addon/login-failed/bad-response-CODE` 99. Connection error trying to login `addon/login-failed/connection-error` @@ -301,7 +302,6 @@ The data collected in error reports includes: * Browser version and User-Agent header * Operating system * The Screenshots system add-on version -* IP address * The exception message * The stack trace * Length of time the add-on was activated diff --git a/docs/export-to-firefox.md b/docs/export-to-firefox.md index b90dd0ee46..a5a323cde0 100644 --- a/docs/export-to-firefox.md +++ b/docs/export-to-firefox.md @@ -7,7 +7,7 @@ Start the system addon release process by copying the following checklist into a - [ ] Bump the minor version number in package.json, following our [version numbering conventions](https://github.com/mozilla-services/screenshots/issues/2647) - [ ] Update changelog: `./bin/generate-commit-log --write stable..master` -- [ ] Create tag: `git tag MAJOR.MINOR.0` +- [ ] Create tag: `git tag MAJOR.MINOR.0` – the version should be higher than the version currently in `package.json` (e.g., if the in-development version is 10.0.0, then tag 10.1.0) - [ ] Push tag: `git push --tags` - [ ] Merge master to stable: `git checkout stable && git merge master && git push` - [ ] Create a Bugzilla release bug, copying [bug 1368146](https://bugzil.la/1368146) diff --git a/server/src/config.js b/server/src/config.js index d39385f6ab..d7dd0aff84 100644 --- a/server/src/config.js +++ b/server/src/config.js @@ -28,13 +28,6 @@ var conf = convict({ env: "CONTENT_ORIGIN", arg: "contentOrigin" }, - extraContentOrigin: { - doc: "If you have a second origin available for migration purposes", - format: String, - default: "", - env: "EXTRA_CONTENT_ORIGIN", - arg: "extraContentOrigin" - }, expectProtocol: { doc: "Treat all incoming requests as using this protocol, instead of defaulting to http: or detecting from X-Forwarded-Proto", format: String, diff --git a/server/src/server.js b/server/src/server.js index b2a35d2ad4..b6e4153b2d 100644 --- a/server/src/server.js +++ b/server/src/server.js @@ -166,7 +166,7 @@ app.set('trust proxy', true); // Disable x-powered-by header app.disable("x-powered-by"); -const CONTENT_NAME = config.contentOrigin; +const CONTENT_NAME = config.contentOrigin || ''; function addHSTS(req, res) { // Note: HSTS will only produce warning on a localhost self-signed cert @@ -199,11 +199,10 @@ app.use((req, res, next) => { } else { dsn = ""; } - let extraContentOrigin = config.extraContentOrigin || ""; req.cspNonce = uuid; res.header( "Content-Security-Policy", - `default-src 'self'; img-src 'self' www.google-analytics.com ${CONTENT_NAME}${extraContentOrigin && ' ' + extraContentOrigin} data:; script-src 'self' www.google-analytics.com 'nonce-${uuid}'; style-src 'self' 'unsafe-inline' https://code.cdn.mozilla.net; connect-src 'self' www.google-analytics.com ${dsn}; font-src https://code.cdn.mozilla.net; frame-ancestors 'none'; object-src 'none';`); + `default-src 'self'; img-src 'self' www.google-analytics.com ${CONTENT_NAME} data:; script-src 'self' www.google-analytics.com 'nonce-${uuid}'; style-src 'self' 'unsafe-inline' https://code.cdn.mozilla.net; connect-src 'self' www.google-analytics.com ${dsn}; font-src https://code.cdn.mozilla.net; frame-ancestors 'none'; object-src 'none';`); res.header("X-Frame-Options", "DENY"); res.header("X-Content-Type-Options", "nosniff"); addHSTS(req, res);