ref #10898
- The redirects configuration's `to` & `from` URL parameters used to ignore it's query string parameters, which resulted in unexpected behavior
- Current changeset only partially fixes the issue. Now `to` URL's query parameters always take precedence over incoming query parameters and the rest of query parameters are passed through.
refs #11085
- Incorrect usage error was logged to the output when there was no recirecst configuration file present in the system. Previously an empty string was returned in such situation, resulting in "ENOENT" error, which was ignored through special handling.
- The fix resembles logic in redirects async getter function where empty array is returned when the config file does not exits.
- Attempting to read unexistent config should not ever happen and will be handled on the config service layer, this is why special "ENOENT" handling has been removed
closes#11085
- Ghost has been using YAML format for other configurations (e.g. routes). The plan is to move to this format for all user-edited settings files. By default JSON format is still used in Ghost Admin API v2/v3, but will be changed to YAML in API v4. Check referenced issue for more context.
- New format supports all the features available before. The main noticeable change is the structure of config file. It is now grouped by redirect HTTP code instead of specifying `"permanent": true | false` attribute for each config property. Example format for YAML config:
```
302:
/from-url/: /to-url/
301:
/category/([a-z0-9\-]+)/i: /tag/$1/
/v([0-9\.]+)/docs/([a-z0-9\-]+)/i: /docs/$2/
```
- Added 2 new endpoints: `POST redirects/upload` and `GET redirects/download`. These serve as an alias to current GET/POST `/redirects/json. "upload/download" naming pattern is introduced to match the convention with other resources that can be uploaded and downloaded (images, themes etc.). `/redirects/json` endpoints will be removed in Admin API v4
- The parsing code from `custom-redirects.js` has been moved to `frontend/services/redirects/settings.js`. This location is more appropriate for this logic and eventually `custom-redirects.js` middlewear might be moved into "frontend" as this middlewear plays a role mostly effecting that area.
no-issue
This issue only occurs when using custom redirects with a subdirectory
setup, and the path to be redirected from is expressed as a regex, and
the url that is being redirected to is not an external url.
The issue has a few components:
- Redirect paths as a regex generally use the ^ to ensure that they
match the beginning of the path.
- The path that the regex is matched against conditionally excludes the
subdirectory, specifically, the subdirectory is excluded for external
urls
These combined means you end up with a regex like /^\/custom-redirect/
and a path like /subdir/custom-redirect, these will not match/replace
correctly, and you'll end in an infinite redirect loop.
The fix here is to *always* remove the subdirectory when testing regex's
and then conditionally adding it back *only* for the redirect, and only
if it is an internal redirect
- Represents that logging is shared across all parts of Ghost at present
* moved core/server/lib/common/logging to core/shared/logging
* updated logging path for generic imports
* updated migration and schema imports of logging
* updated tests and index logging import
* 🔥 removed logging from common module
* fixed tests
* moved `server/config` to `shared/config`
* updated config import paths in server to use shared
* updated config import paths in frontend to use shared
* updated config import paths in test to use shared
* updated config import paths in root to use shared
* trigger regression tests
* of course the rebase broke tests
- Added a wrapper around express.Router to our shared/express util
- Also export static and _express
- Use this shared util everywhre, meaning express is only used directly in this one file
- ATM this file is mostly an experiment / debug helper, it might be removed again later
- The aim is to have a minimal framework wrapping express that allows us to:
- reduce our usage of express() in favour of Router()
- unify some of our duplicated logic
- fix some structural issues e.g. Sentry
- make it easier to understand the codebase
refs #10898
- Execute string replacement on external paths
- Take non-top-level base URLs into consideration (to avoid #10776 dups)
- Added tests for all of the above cases
- All var declarations are now const or let as per ES6
- All comma-separated lists / chained declarations are now one declaration per line
- This is for clarity/readability but also made running the var-to-const/let switch smoother
- ESLint rules updated to match
How this was done:
- npm install -g jscodeshift
- git clone https://github.com/cpojer/js-codemod.git
- git clone git@github.com:TryGhost/Ghost.git shallow-ghost
- cd shallow-ghost
- jscodeshift -t ../js-codemod/transforms/unchain-variables.js . -v=2
- jscodeshift -t ../js-codemod/transforms/no-vars.js . -v=2
- yarn
- yarn test
- yarn lint / fix various lint errors (almost all indent) by opening files and saving in vscode
- grunt test-regression
- sorted!
no issue
- Handlebars now throws an Error for misuse errors within the if/unless
helpers, but our error handling checks for a TypeError
- this would skip using an IncorrectUsageError and ends up throwing a GhostError
- this commit removes the TypeError check and switches to using the
Handlebars error message
- added core/shared to watched folders in grunt
- moved sentry to shared
- moved express initialisation to a shared file
- always set trust proxy + sentry error handler
- use this new express init everywhere, and remove duplicate trust proxy and sentry error handler code
closes#11766, refs 7284227f1
- when we changed from host to hostname, more changed than just using the x-forwarded-host if trusted because express req.hostname does not return the port
- this causes issues with an infinite redirect if you try to set a different admin host with a port
- added a test to demonstrate the case, that didn't fail due to an error in the test logic
- switched from redirecting based on req.hostname to using req.vhost.host which has the correct trusted, requested value that we should rely on
- simplified the comparison logic to explicitly compare host with host
- This code was a little verbose, which made it hard to see what was happening (it still is a bit)
- Used destructuring to reduce the code
- Renamed a few variables
- we had urlRedirects, urlRedirects.adminRedirect and adminRedirects
- all do kinda similar things, but for different contexts so for now I've done a minimal renaming for clarity
- and updated some comments!!
- also removed totally unnecessary if res.isAdmin clause, as we don't use that, and it was never true
- cache-control had some logic in it for private blogging + similar logic exists for members in site/app
- having it in 2 places is weird, and having it inside the mw makes the mw less generic/reusable
- instead of requiring config inside the middleware, we pass config in for the one case where this is used
- fixed tests that didn't test anything 🙈
- Moved normalize image mw from shared to api as it is not shared (except within the API)
- This file is only used in one part of the app, this updates the code structure to reflect this
- This is one of many similar changes needed to make it easier to refactor to the existing setup
- Moved upload validation mw from shared to api as it is not shared (except within the API)
- Co-located the code with the upload middleware, as it's small and gives us a nice API of .upload.single and .upload.validation
- This file is only used in one part of the app, this updates the code structure to reflect this
- This is one of many similar changes needed to make it easier to refactor to the existing setup
- These two functions have no dependencies and are only used in valiation/upload
- Co-locating the code makes it easier to move
- Exported them with a new module.exports._test pattern - we'll see about whether this is a good idea
- This is one of many similar changes needed to make it easier to refactor to the existing setup
- Moved upload mw from shared to api as it is not shared (except within the API)
- This file is only used in one part of the app, this updates the code structure to reflect this
- This is one of many similar changes needed to make it easier to refactor to the existing setup
- Moved update-user-last-seen from shared to api as it is not shared (except within the API)
- This file is only used in one part of the app, this updates the code structure to reflect this
- This is one of many similar changes needed to make it easier to refactor to the existing setup
- Moved api cors from shared to api as it is not shared (except within the API)
- This file is only used in one part of the app, this updates the code structure to reflect this
- This is one of many similar changes needed to make it easier to refactor to the existing setup
- Moved version-match from shared to api as it is not shared (except within the API)
- This file is only used in one part of the app, this updates the code structure to reflect this
- This is one of many similar changes needed to make it easier to refactor to the existing setup
- Moved ghost-locals from shared to parent as it is not shared
- This file is only used in one place, this updates the code structure to reflect this
- This is one of many similar changes needed to make it easier to refactor to the existing setup
- This allows shared middleware to be unhooked from the parent app
- Moved emit-events from shared to parent as it is not shared
- This file is only used in one place, this updates the code structure to reflect this
- This is one of many similar changes needed to make it easier to refactor to the existing setup
- Moved log-request from shared to parent as it is not shared
- This file is only used in one place, this updates the code structure to reflect this
- This is one of many similar changes needed to make it easier to refactor to the existing setup
- Moved request-id from shared to parent as it is not shared
- This file is only used in one place, this updates the code structure to reflect this
- This is one of many similar changes needed to make it easier to refactor to the existing setup
- Moved serve-public-file from shared to site as it is not shared
- This file is only used in one place, this updates the code structure to reflect this
- This is one of many similar changes needed to make it easier to refactor to the existing setup
- Moved handle-image-sizes from shared to site as it is not shared
- This file is only used in one place, this updates the code structure to reflect this
- This is one of many similar changes needed to make it easier to refactor to the existing setup
- Moved static-theme from shared to site as it is not shared
- This file is only used in one place, this updates the code structure to reflect this
- This is one of many similar changes needed to make it easier to refactor to the existing setup
- Moved serve-favicon from shared to site as it is not shared
- This file is only used in one place, this updates the code structure to reflect this
- This is one of many similar changes needed to make it easier to refactor to the existing setup
- Moved admin-redirects from shared to site as it is not shared
- This file is only used in one place, this updates the code structure to reflect this
- This is one of many similar changes needed to make it easier to refactor to the existing setup
fixes#11740
- there was a discrepancy in the use of `private_blog` within the
code, and `private_block` in the default config
- this commit switches the code to `block` in order to avoid breaking
existing configs
- moved image.manipulation lib to a new package called @tryghost/image-transform
- new package has an updated API signature, so the method calls have changed but the underlying code is identical
- removed the optional sharp dependency from Ghost, as this is now optionally required by the image-transform module
no issue
- some errors weren't being reported because they were being passed to
Sentry before our middleware could populate the error information
- this commit inserts the Sentry middleware into these steps
no issue
- when `servePublicFile` middleware serves an image it resulted in a "Cannot set headers after they are sent to the client" error because `next()` was erroneously called for successful requests which then tripped the `prettyUrls` middleware which tries to perform a redirect
- only calling `next()` when an error is present allows errors to be picked up by later middleware but successful requests end in the `servePublicFile` middleware
no issue
- serving of our public asset images was broken
- we were reading the binary file in as a string so we could do url transforms, this meant data was lost/corrupted and browsers could not display the served data
- we were using the wrong mime-type for pngs which meant browsers were triggering downloads rather than displaying images (at least when accessed directly)
- updates uses of `servePublicFile` to have the correct png mimetype
- adjusts `servePublicFile` to treat any mime type starting with `image` as a binary file, passing the file directly through express using `res.sendFile` and skipping the in-memory content caching which is mostly only useful for text files with URL transforms
no issue
- if a request was sent for an resized image URL that didn't contain a
file extension, the code would eventually end up throwing a 500
- this commit checks for this case and returns a 404
no issue
- requests for resized images with a trailing slash would end up
throwing a EISDIR error because it got through to writing an
image buffer to a directory
- we want to cut this off early and disallow trailing slashes
no issue
When using certain proxy setups that result in `host` and `x-forwarded-host` being different, it became impossible to access Ghost because all routes showed generic 404 pages.
- `vhost` module that we are using to separate front-end and admin urls does not use express' `req.hostname` so it does not pick up the `x-forwarded-host` url that express' `'trust proxy'` config gives us
- switched to the forked `@tryghost/vhost-middleware` package which has a one-line change to use `req.hostname || req.host`
- added `'trust proxy'` config to the admin express app and switched to using `req.hostname` in our redirect code to avoid infinite redirect loops
refs https://github.com/TryGhost/Ghost/pull/11152
- Added subscribers table drop migration
- Removed subscribers from schema
- Removed subscribers controllers/routes/regression tests
- Removed subscriber related API code
- Removed subscribers from internal apps
- Removed subscriber importer
- Removed subscriber model
- Removed subscriber related permissions
- Removed webhook code related to subscribers
- When upgrading to v3 it is on the site admin to migrate all zapps or any other webhook clients to use members
- Removed subscriber-specific translation
- Removed subscriber lab flag
- Currently, we create a request ID for internal use if one isn't set & this is used in logs
- If a custom request ID is set via X-Request-ID header, this gets logged, however, we don't return this with the response
- Means that a custom ID gets lost on the way back out, and makes tracing requests through a system trickier
- This change ensures that if X-Request-ID is set on the request, it is also set on the response so that requests can be properly traced
- It's easy to set this in e.g. nginx so that the feature becomes available - Ghost doens't need to do this
- Note: also split request id handling out into new middleware
