Use "wp_die" template message instead of plain message when Atom is disabled.

[Simek]

Use "wp_die" template message instead of plain message when Atom is disabled.

[DD32]

The AtomServer::forbidden() function is where you should probably do that.

wp_die($reason, __('Atom Feed'), array('response' => 403) );

bundled inside that function would take care of it?

[westi]

Indeed if we are to wp_die it should be inside forbidden.

We also need to consider the other error cases here and how this change will possibly affect clients.

[jacobsantos]

Based off of r10258

[westi]

josephscott - is this change likely to break APP clients or is it safe to commit?

[Denis-de-Bernardy]

not safe imo. moving to future release, as this really needs some testing.

[hakre]

Tested the patch with various clients over HTTP which is the prefered tranport protocoll. Looks very good, the Status Header is transmitted as it must be, it gives a 403 forbidden message. Looks all perfectly well and to assume that this will create incompabilities must be a myth.

[westi]

I discussed this with the client developers on the wp-xmlrpc list and we feel that it would be better to have a parsable xml response here so that clients can parse it out.

However, there doesn't appear to be any provision for this in the APP RFC - there is no detail spec on error responses :-(

Moving it to future for now until we have a good workable patch in that direction.

[hakre]

Replying to westi:

I discussed this with the client developers on the wp-xmlrpc list and we feel that it would be better to have a parsable xml response here so that clients can parse it out.

Thanks for sharing your feelings but I doubt that this is in the scope of the ticket, so let's stick with the basics.

However, there doesn't appear to be any provision for this in the APP RFC - there is no detail spec on error responses :-(

Well, the RFC I found (rfc5023) is perfectly clear about the issue, I wonder where you both might have looked:

5.5.  Use of HTTP Response Codes

   The Atom Protocol uses the response status codes defined in HTTP to
   indicate the success or failure of an operation.  Consult the HTTP
   specification [RFC2616] for detailed definitions of each status code.

   Implementers are asked to note that according to the HTTP
   specification, HTTP 4xx and 5xx response entities SHOULD include a
   human-readable explanation of the error.

​http://tools.ietf.org/html/rfc5023#section-5.5

Moving it to future for now until we have a good workable patch in that direction.

I guess you just made a mistake looking up the specs so I revert your changes. As stated above having a 403 with a human readable response (HTML) should be favored over XML. The patch looks perfectly well to me, it actually is the implementation of that RFC. Having an empty body in the response as of now is the actual bug.

[westi]

That is exactly the spec I read - and it is very vague.

I also read a lot more information on this - an error response format was discussed at committee stage but did not progress further.

What we have now is better than html for an APP client to use - ideally a well formed parse-able XML response seems the best route for an even friendlier client response.

[hakre]

Replying to westi:

That is exactly the spec I read - and it is very vague.

vague? Next to referring other RFCs it ...

I also read a lot more information on this - an error response format was discussed at committee stage but did not progress further.

(then there ain't one)

What we have now is better than html for an APP client to use - ideally a well formed parse-able XML response seems the best route for an even friendlier client response.

... was clearly written that not a machine but a human readable response should be given.

[westi]

Closing this down as we haven't seen much call for it and I haven't seen a strong uptake of APP by client developers.