From f5f02d71688e337ee48a98baae993f4b1c10e7b3 Mon Sep 17 00:00:00 2001 From: Kevin Ansfield Date: Tue, 23 Nov 2021 10:58:38 +0000 Subject: [PATCH 1/6] Fixed errors being swallowed in oembed service no issue When switching the oembed service to async/await the error handling was not correctly refactored. `this.errorHandler(url)` was returning a curried function so it could be used as `.catch(this.errorHandler(url))` but that's not how it's being used after the async/await change meaning we were returning a function rather than the result of that function. - `this.errorHandler(url)` is now only used in one place where `url` is available so removed the method and moved the body of the curried function inline into the `catch` handler - added a message to the logged error so it's more clear what the log refers to --- core/server/services/oembed.js | 50 ++++++++++++++++------------------ 1 file changed, 23 insertions(+), 27 deletions(-) diff --git a/core/server/services/oembed.js b/core/server/services/oembed.js index e36136dc72..d5a3bf5c95 100644 --- a/core/server/services/oembed.js +++ b/core/server/services/oembed.js @@ -63,6 +63,7 @@ class OEmbed { */ constructor({config, externalRequest}) { this.config = config; + /** @type {IExternalRequest} */ this.externalRequest = async (url, requestConfig) => { if (this.isIpOrLocalhost(url)) { @@ -74,6 +75,7 @@ class OEmbed { } return response; }; + /** @type {ICustomProvider[]} */ this.customProviders = []; } @@ -108,27 +110,6 @@ class OEmbed { } } - /** - * @param {string} url - */ - errorHandler(url) { - /** - * @param {Error|errors.GhostError} err - */ - return async (err) => { - // allow specific validation errors through for better error messages - if (errors.utils.isIgnitionError(err) && err.errorType === 'ValidationError') { - throw err; - } - - // log the real error because we're going to throw a generic "Unknown provider" error - logging.error(new errors.GhostError({err})); - - // default to unknown provider to avoid leaking any app specifics - return this.unknownProvider(url); - }; - } - async fetchBookmarkData(url) { const metascraper = require('metascraper')([ require('metascraper-url')(), @@ -300,10 +281,9 @@ class OEmbed { * @returns {Promise} */ async fetchOembedDataFromUrl(url, type) { - let data; - try { const urlObject = new URL(url); + for (const provider of this.customProviders) { if (await provider.canSupportRequest(urlObject)) { const result = await provider.getOEmbedData(urlObject, this.externalRequest); @@ -313,23 +293,39 @@ class OEmbed { } } + // fetch only bookmark when explicitly requested if (type === 'bookmark') { return this.fetchBookmarkData(url); } - data = await this.fetchOembedData(url); + // attempt to fetch oembed + let data = await this.fetchOembedData(url); + // fallback to bookmark when we can't get oembed if (!data && !type) { data = await this.fetchBookmarkData(url); } + // couldn't get anything, throw a validation error if (!data) { - data = await this.unknownProvider(url); + return this.unknownProvider(url); } return data; - } catch (e) { - return this.errorHandler(url); + } catch (err) { + // allow specific validation errors through for better error messages + if (errors.utils.isIgnitionError(err) && err.errorType === 'ValidationError') { + throw err; + } + + // log the real error because we're going to throw a generic "Unknown provider" error + logging.error(new errors.GhostError({ + message: 'Encountered error when fetching oembed', + err + })); + + // default to unknown provider to avoid leaking any app specifics + return this.unknownProvider(url); } } } From 15d590554965d3aa0f3e25644a63e7670116e6ed Mon Sep 17 00:00:00 2001 From: Daniel Lockyer Date: Thu, 25 Nov 2021 11:40:23 +0100 Subject: [PATCH 2/6] =?UTF-8?q?=F0=9F=90=9B=20Fixed=20intermittent=20failu?= =?UTF-8?q?res=20with=20embedding?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit refs https://github.com/TryGhost/Team/issues/1235 - we are seeing `oembed-parser` 1.5.2 have intermittent issues when fetching oembed data - we're not sure of the reason but reverting the dependency to 1.4.9 seems to fix the issue - this commit reverted the bump in Ghost and adds it to Renovate's ignore list so it isn't automatically bumped in the future --- package.json | 2 +- renovate.json | 1 + yarn.lock | 24 ++++++++++++++++++------ 3 files changed, 20 insertions(+), 7 deletions(-) diff --git a/package.json b/package.json index 3e6f1c927b..2244ab5ef2 100644 --- a/package.json +++ b/package.json @@ -159,7 +159,7 @@ "mysql": "2.18.1", "nconf": "0.11.3", "node-jose": "2.0.0", - "oembed-parser": "1.5.2", + "oembed-parser": "1.4.9", "passport": "0.5.0", "passport-google-oauth": "2.0.0", "path-match": "1.2.4", diff --git a/renovate.json b/renovate.json index 19bea5e414..1073b6dcdc 100644 --- a/renovate.json +++ b/renovate.json @@ -11,6 +11,7 @@ "intl-messageformat", "moment", "moment-timezone", + "oembed-parser", "simple-dom" ], "ignorePaths": ["test"], diff --git a/yarn.lock b/yarn.lock index 7fe85f39af..cd31ebb14b 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3757,6 +3757,13 @@ cron-validate@^1.4.1, cron-validate@^1.4.3: dependencies: yup "0.32.9" +cross-fetch@^3.1.4: + version "3.1.4" + resolved "https://registry.yarnpkg.com/cross-fetch/-/cross-fetch-3.1.4.tgz#9723f3a3a247bf8b89039f3a380a9244e8fa2f39" + integrity sha512-1eAtFWdIubi6T4XPy6ei9iUFoKpUkIF971QLN8lIvvvwueI65+Nw5haMNKUwfJxabqlIIDODJKGrQ66gxC0PbQ== + dependencies: + node-fetch "2.6.1" + cross-spawn@^6.0.5: version "6.0.5" resolved "https://registry.yarnpkg.com/cross-spawn/-/cross-spawn-6.0.5.tgz#4a5ec7c64dfae22c3a14124dbacdee846d80cbc4" @@ -5683,7 +5690,7 @@ got@9.6.0, got@^9.6.0: to-readable-stream "^1.0.0" url-parse-lax "^3.0.0" -got@^11.8.2, got@~11.8.2: +got@~11.8.2: version "11.8.2" resolved "https://registry.yarnpkg.com/got/-/got-11.8.2.tgz#7abb3959ea28c31f3576f1576c1effce23f33599" integrity sha512-D0QywKgIe30ODs+fm8wMZiAcZjypcCodPNuMz5H9Mny7RJ+IjJ10BdmGW7OM7fHXP+O7r6ZwapQ/YQmMSvB0UQ== @@ -8447,6 +8454,11 @@ node-addon-api@^4.2.0: resolved "https://registry.yarnpkg.com/node-addon-api/-/node-addon-api-4.2.0.tgz#117cbb5a959dff0992e1c586ae0393573e4d2a87" integrity sha512-eazsqzwG2lskuzBqCGPi7Ac2UgOoMz8JVOXVhTvvPDYhthvNpefx8jWD8Np7Gv+2Sz0FlPWZk0nJV0z598Wn8Q== +node-fetch@2.6.1: + version "2.6.1" + resolved "https://registry.yarnpkg.com/node-fetch/-/node-fetch-2.6.1.tgz#045bd323631f76ed2e2b55573394416b639a0052" + integrity sha512-V4aYg89jEoVRxRb2fJdAg8FHvI7cEyYdVAh94HH0UIK8oJxUfkjlDQN9RbMx+bEjP7+ggMiFRprSti032Oipxw== + node-fetch@^2.6.0: version "2.6.6" resolved "https://registry.yarnpkg.com/node-fetch/-/node-fetch-2.6.6.tgz#1751a7c01834e8e1697758732e9efb6eeadfaf89" @@ -8758,12 +8770,12 @@ object.pick@^1.2.0, object.pick@^1.3.0: dependencies: isobject "^3.0.1" -oembed-parser@1.5.2: - version "1.5.2" - resolved "https://registry.yarnpkg.com/oembed-parser/-/oembed-parser-1.5.2.tgz#a35830793f0cf03519ffedb9a43b815d291f0327" - integrity sha512-Um9aqB6fu1+DmxyKawuVcP4TpNv7gw9jmd65mIVxkQxR40+BpEz2jrl/0xaahV67omOl9eGv/OxtK0O7zVh5JQ== +oembed-parser@1.4.9: + version "1.4.9" + resolved "https://registry.yarnpkg.com/oembed-parser/-/oembed-parser-1.4.9.tgz#d23127c96185dfa2d998b8567e6c7df164f3d824" + integrity sha512-RCOjuv20IMm9XekZB1ZefdYPc+x5qe8IyCeY/xcN71I2z70vQl+L420eI5Mjyy/NQFLv+QPEgj/aYh1vptO83w== dependencies: - got "^11.8.2" + cross-fetch "^3.1.4" on-finished@^2.3.0, on-finished@~2.3.0: version "2.3.0" From 483ba3e0f99b6a8901e4d16bd8deb0063bdb3323 Mon Sep 17 00:00:00 2001 From: Fabien 'egg' O'Carroll Date: Thu, 25 Nov 2021 12:33:22 +0200 Subject: [PATCH 3/6] Truncated offers.name to 40 characters (#13781) refs https://github.com/TryGhost/Team/issues/1236 We use Offer names for the Stripe Coupon name - which has a limit of 40 characters. We are now introducing a limit of 40 characters to Offer names too. This migration ensures that all our data in the DB is valid. --- .../versions/4.23/01-truncate-offer-names.js | 58 +++++++++++++++++++ 1 file changed, 58 insertions(+) create mode 100644 core/server/data/migrations/versions/4.23/01-truncate-offer-names.js diff --git a/core/server/data/migrations/versions/4.23/01-truncate-offer-names.js b/core/server/data/migrations/versions/4.23/01-truncate-offer-names.js new file mode 100644 index 0000000000..ddf5fe62d2 --- /dev/null +++ b/core/server/data/migrations/versions/4.23/01-truncate-offer-names.js @@ -0,0 +1,58 @@ +const logging = require('@tryghost/logging'); +const {createTransactionalMigration} = require('../../utils'); + +/** + * @param {(val: string) => boolean} exists + * @param {string} requested + * @param {string} attempt + * @param {number} n + * + * @returns {string} + */ +function getUnique(exists, requested, attempt = requested, n = 1) { + if (!exists(attempt)) { + return attempt; + } + const newAttempt = requested.slice(0, -n.toString().length) + n; + return getUnique(exists, requested, newAttempt, n + 1); +} + +module.exports = createTransactionalMigration( + async function up(knex) { + const allOffers = await knex + .select('id', 'name') + .from('offers'); + + const offersNeedingTrunctation = allOffers.filter((row) => { + return row.name.length >= 40; + }); + + if (offersNeedingTrunctation.length === 0) { + logging.warn('No Offers found needing truncation'); + return; + } else { + logging.info(`Found ${offersNeedingTrunctation.length} Offers needing truncation`); + } + + const truncatedOffers = offersNeedingTrunctation.reduce((offers, row) => { + function exists(name) { + return offers.find(offer => offer.name === name) !== undefined; + } + + const updatedRow = { + id: row.id, + name: getUnique(exists, row.name.slice(0, 40)) + }; + + return offers.concat(updatedRow); + }, []); + + for (const truncatedOffer of truncatedOffers) { + await knex('offers') + .update('name', truncatedOffer.name) + .where('id', truncatedOffer.id); + } + }, + // no-op we've lost the data required to roll this back + async function down() {} +); From 9730081590f351b90fe5412b41400d64cd0d9219 Mon Sep 17 00:00:00 2001 From: Fabien O'Carroll Date: Thu, 25 Nov 2021 12:53:57 +0200 Subject: [PATCH 4/6] =?UTF-8?q?=F0=9F=90=9B=20Restricted=20Offer=20name=20?= =?UTF-8?q?to=2040=20characters?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit refs https://github.com/TryGhost/Team/issues/1236 We want to ensure that Offers share a name with the correspondent coupon in Stripe, which have a max length of 40 characters, so we are applying the same restriction to Offers. --- package.json | 4 ++-- yarn.lock | 28 ++++++++++++++-------------- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/package.json b/package.json index 2244ab5ef2..0fed5be6b2 100644 --- a/package.json +++ b/package.json @@ -79,10 +79,10 @@ "@tryghost/limit-service": "1.0.0", "@tryghost/logging": "1.0.1", "@tryghost/magic-link": "1.0.14", - "@tryghost/members-api": "2.7.4", + "@tryghost/members-api": "2.7.5", "@tryghost/members-csv": "1.1.8", "@tryghost/members-importer": "0.3.4", - "@tryghost/members-offers": "0.10.2", + "@tryghost/members-offers": "0.10.3", "@tryghost/members-ssr": "1.0.15", "@tryghost/metrics": "1.0.1", "@tryghost/minifier": "0.1.1", diff --git a/yarn.lock b/yarn.lock index cd31ebb14b..59a8f99ea9 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1866,10 +1866,10 @@ "@tryghost/domain-events" "^0.1.3" "@tryghost/member-events" "^0.3.1" -"@tryghost/members-api@2.7.4": - version "2.7.4" - resolved "https://registry.yarnpkg.com/@tryghost/members-api/-/members-api-2.7.4.tgz#0941f8d892fb72c15f820947b2ac444bd570cd6d" - integrity sha512-C7w/aKRzDwR3Aol0mVborOQAv6mE8cVV4G0J2L757BjhyKuMeVAOb6X+uCYBLNhe0zabiKow6GwN+f1M/qXp4A== +"@tryghost/members-api@2.7.5": + version "2.7.5" + resolved "https://registry.yarnpkg.com/@tryghost/members-api/-/members-api-2.7.5.tgz#693aad037697757e8c5f69f267606208bbb9c19f" + integrity sha512-KieQlWshDAUG3CxHjN1J8k3IH5tl/oCDEB7SM3nnbZoVV2frSnFInAHXCLFwkjunGx0dpJEdzVUgr/VdNlb9DA== dependencies: "@tryghost/debug" "^0.1.2" "@tryghost/domain-events" "^0.1.3" @@ -1879,7 +1879,7 @@ "@tryghost/member-analytics-service" "^0.1.4" "@tryghost/member-events" "^0.3.1" "@tryghost/members-analytics-ingress" "^0.1.5" - "@tryghost/members-payments" "^0.1.4" + "@tryghost/members-payments" "^0.1.5" "@tryghost/members-stripe-service" "^0.5.0" "@tryghost/tpl" "^0.1.2" "@types/jsonwebtoken" "^8.5.1" @@ -1914,21 +1914,21 @@ "@tryghost/tpl" "^0.1.3" moment-timezone "0.5.23" -"@tryghost/members-offers@0.10.2", "@tryghost/members-offers@^0.10.2": - version "0.10.2" - resolved "https://registry.yarnpkg.com/@tryghost/members-offers/-/members-offers-0.10.2.tgz#5716e9be902ef924c3fb9fad48a7ab38cd58b7c0" - integrity sha512-LL+Rrn5VHwGf8cMnIVCFhkkBI6Eg1CkVjLa3qi0/NRncy28IhVn0jjg1aJa6hoicCWZLg1/efT2RhI5RvC4Gzw== +"@tryghost/members-offers@0.10.3", "@tryghost/members-offers@^0.10.3": + version "0.10.3" + resolved "https://registry.yarnpkg.com/@tryghost/members-offers/-/members-offers-0.10.3.tgz#759cf2c0650b09309e9295f178aff17990d29d5c" + integrity sha512-W0lGfOfJkRrESxBEgkBW5XqQuhW/5Cj+QIzOb2b6nT8u3VCE4dX0VCkLWrc6HJq+Hpgv0potmGOLoj37xvFmEw== dependencies: "@nexes/mongo-utils" "^0.3.1" "@tryghost/string" "^0.1.20" -"@tryghost/members-payments@^0.1.4": - version "0.1.4" - resolved "https://registry.yarnpkg.com/@tryghost/members-payments/-/members-payments-0.1.4.tgz#613e588b354a3e432f159f9179c946d90388bf85" - integrity sha512-0fA0uLRV5zPaZgEl7XuQYqtLZjJWWJVUXZq2xiKNi5jw4/koCPKjVicBOuMtRWBK0Nn5Mma6B9uBvhkMGUMqgw== +"@tryghost/members-payments@^0.1.5": + version "0.1.5" + resolved "https://registry.yarnpkg.com/@tryghost/members-payments/-/members-payments-0.1.5.tgz#9d7f7f48e118a836c5825e19403dc4c0611d4976" + integrity sha512-iT61ffRqVH89KUOUtzDrXTn7fG38/LUcKFaFgrnWhTwcVdcglKefE2cWAjm25BNcYKM3lHJFxG/3PTVHmcOvTw== dependencies: "@tryghost/domain-events" "^0.1.3" - "@tryghost/members-offers" "^0.10.2" + "@tryghost/members-offers" "^0.10.3" "@tryghost/members-ssr@1.0.15": version "1.0.15" From de75b26976c84832e0ba834d6c05b95029871f4b Mon Sep 17 00:00:00 2001 From: Daniel Lockyer Date: Thu, 25 Nov 2021 11:16:29 +0000 Subject: [PATCH 5/6] Updated Admin to v4.23.0 --- core/client | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/client b/core/client index 18b5ca27f6..96a53702dd 160000 --- a/core/client +++ b/core/client @@ -1 +1 @@ -Subproject commit 18b5ca27f64c5943471f60ecc6e56d2e1d09d72c +Subproject commit 96a53702ddfe7f12e7bd5b374f499848e9c2be70 From 0fe59026d0c3f96c9f5fa0bfdf8a81ebaa566cea Mon Sep 17 00:00:00 2001 From: Daniel Lockyer Date: Thu, 25 Nov 2021 11:16:29 +0000 Subject: [PATCH 6/6] v4.23.0 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 0fed5be6b2..cb5c2926c4 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "ghost", - "version": "4.22.4", + "version": "4.23.0", "description": "The professional publishing platform", "author": "Ghost Foundation", "homepage": "https://ghost.org",