Skip to content

[5.3] Set http status header for XML/feed responses #45419

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

Open
wants to merge 2 commits into
base: 5.3-dev
Choose a base branch
from

Conversation

ManuelHu
Copy link
Contributor

Pull Request for Issue #45398.

Summary of Changes

set the error code on the HTTP response

this is similar to the other error renderes (i.e. copied from https://github.com/joomla/joomla-cms/blob/5.3-dev/libraries/src/Error/Renderer/JsonRenderer.php)

Testing Instructions

  1. set a menu item that shows an article as the home menu item
    2, open https://<your-site>/?format=feed -- of course, this is not a useful feed link (I know that; thats the whole point.

  2. see a response such as

<?xml version="1.0" encoding="UTF-8"?>
<error>
	<code>404</code>
	<message>View nicht gefunden [Name, Typ, Präfix]: article, feed, site</message>
</error>
  1. open the browser DevTools to see the http status code reported from the server

Actual result BEFORE applying this Pull Request

the HTTP response status is 200

Expected result AFTER applying this Pull Request

the HTTP response status (not the one listed as <code>404</code>) is actually 404

Link to documentations

Please select:

  • Documentation link for docs.joomla.org:

  • No documentation changes for docs.joomla.org needed

  • Pull Request link for manual.joomla.org:

  • No documentation changes for manual.joomla.org needed

@richard67 richard67 added the bug label Apr 30, 2025
@richard67 richard67 changed the title set http status header for XML/feed responses [5.3] Set http status header for XML/feed responses Apr 30, 2025
@brianteeman
Copy link
Contributor

Not sure but is it not enough to add this single line in the function

\Joomla\CMS\Factory::getApplication()->setHeader('status', $error->getCode());

@ManuelHu
Copy link
Contributor Author

well, yes, if we assume that

  • XML is only ever sent over HTTP (i.e. CLIApplication has no setHeader method)
    I am not sure if this is the case?
  • and that every exception sets a HTTP status (which is not the case). The default value is 0, and 0 is not a valid HTTP status

note: a similar check is not only implemented in JsonRenderer, but also in ErrorDocument

$status = $this->_error->getCode();
if ($status < 400 || $status > 599) {
$status = 500;
}
$errorReporting = CmsFactory::getApplication()->get('error_reporting');
if ($errorReporting === "development" || $errorReporting === "maximum") {
$status .= ' ' . str_replace("\n", ' ', $this->_error->getMessage());
}
CmsFactory::getApplication()->setHeader('status', $status);

also in the joomla-framework/application documentation:
https://github.com/joomla-framework/application/blob/e063a4a308729b930d853e29a9f7c303cac68629/README.md?plain=1#L54

However, this check is missing in other places. Also note, I am not an export in this (all of this just based on my code reviw of the last day), so I am not really sure what to decide here.

@laoneo
Copy link
Member

laoneo commented May 5, 2025

The instanceof check is fine, but I think ->setHeader('status', $error->getCode() < 400 ? 500 : $error->getCode()); would be enough. Or do you see an issue with it?

@ManuelHu
Copy link
Contributor Author

ManuelHu commented May 5, 2025

No, this should be equivalent in all typical cases. This very verbose code is just the same as already in the Json response, but I can certainly simplify the code.

@ceford
Copy link
Contributor

ceford commented May 24, 2025

I have tested this item ✅ successfully on fbc2426

I see the network response code change from 200 to 404 after applying the patch and the Firefox Browser Tools show a problem icon. So it works as described but I can't comment on the experts comments.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/45419.

@exlemor exlemor added the PBF Pizza, Bugs and Fun label Aug 23, 2025
@MacJoom
Copy link
Contributor

MacJoom commented Aug 23, 2025

I have tested this item ✅ successfully on fbc2426

On 5.3.3 - Status changed to 404 after applying Patch


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/45419.

@exlemor
Copy link

exlemor commented Aug 23, 2025

I have tested this item ✅ successfully on 8ab15d6

I have tested this successfully (thanks @MacJoom), @ManuelHu


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/45419.

@richard67
Copy link
Member

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/45419.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Aug 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug PBF Pizza, Bugs and Fun PR-5.3-dev RTC This Pull Request is Ready To Commit
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants