-
Notifications
You must be signed in to change notification settings - Fork 438
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Return 404 Not Found status code on missing identifiers #2816
Return 404 Not Found status code on missing identifiers #2816
Conversation
8241a66
to
26ace07
Compare
26ace07
to
ab90c7a
Compare
Thanks @jensvannerum. I tested this on localhost with DSpace 7.6.2-SNAPSHOT (note that the frontend must be built in production mode) and it works. I checked a variety of URLs that were previously returning HTTP 200 incorrectly: $ curl --head localhost:4000/handle/foo/bar
HTTP/1.1 404 Not found
$ curl --head localhost:4000/handle/10568/3 <--- this exists, and is expected to redirect
HTTP/1.1 301 Moved Permanently
$ curl --head localhost:4000/handle/10568/3/browse
HTTP/1.1 404 Not found
$ curl --head localhost:4000/handle/10000000/093991991
HTTP/1.1 404 Not found I will let someone else review the code. |
Hi @jensvannerum, |
…atus-code-object-not-found-7.4
Hi @jensvannerum, |
…us-code-object-not-found-7.4
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Thanks @jensvannerum ! I've verified this works and it fixes the bug where some of these "not found" URLs were returning 200 OK
Backport failed for Please cherry-pick the changes locally and resolve any conflicts. git fetch origin dspace-7_x
git worktree add -d .worktree/backport-2816-to-dspace-7_x origin/dspace-7_x
cd .worktree/backport-2816-to-dspace-7_x
git switch --create backport-2816-to-dspace-7_x
git cherry-pick -x ab90c7a733e575c9a75407be549d124bff49fd48 f9c8103266f7ca392356be92107d0e9d8bb7f89c |
Manually ported to |
References
Add references/links to any related issues or PRs. These may include:
Description
This change makes sure the correct status code is returned for pages related to missing identifiers
Instructions for Reviewers
This PR simply uses the
ServerResponseService
to set the correct status code on initialisation of the component..Testing this is possible by sending a curl request to the issues mentioned in the original issue:
And verify 404 is returned instead of 200
Checklist
This checklist provides a reminder of what we are going to look for when reviewing your PR. You need not complete this checklist prior to creating your PR (draft PRs are always welcome). If you are unsure about an item in the checklist, don't hesitate to ask. We're here to help!
yarn lint
yarn check-circ-deps
)package.json
), I've made sure their licenses align with the DSpace BSD License based on the Licensing of Contributions documentation.