Returning 200 Ok instead of 200 OK

[Tom Haddon]

Zope appears to be returning a 200 Ok instead of 200 OK HTTP return code. I was made aware of this by an external monitoring organisation that was looking for 200 OK and so was reporting our site as being unavailable.

Appears to be a problem at line 671 of /src/zope/publisher/http.py of Zope3's trunk in the setStatus function. Also, it looks as if this should be getting it's text value from the status_reasons dictionary, rather than defining it separately.

[Elliot Murphy (statik)]

Attached is a patch which fixes the problem and updates two tests that were affected. The patch is against:
URL: svn://svn.zope.org/repos/main/Zope3/trunk
Repository Root: svn://svn.zope.org/repos/main
Repository UUID: 62d5b8a3-27da-0310-9561-8e5933582275
Revision: 75055
Node Kind: directory
Schedule: normal
Last Changed Author: ctheune
Last Changed Rev: 75055
Last Changed Date: 2007-05-03 11:56:44 -0400 (Thu, 03 May 2007)

Thanks to stevea for suggesting how to fix it.

[Dmitry Vasiliev (hdima)]

Fixed on the trunk at revision 75062

[Dmitry Vasiliev (hdima)]

Fixed on the Zope 3 trunk at revision 75062

[Elliot Murphy (statik)]

Need to apply this to the zope servers that launchpad is running

[Martijn Pieters (mjpieters)]

Uhm, I know this is now fixed, but why was this set to a 'High' urgency?? The RFC states that the response code needs to recognized, but that the exact reason phrase wording is a recommendation only and can be localized. In other words, if I return HTTP/1.1 200 All is fine, Dude! I'd still be RFC compliant. The monitoring app is badly broken if it matches against the phrase!

See http://www.w3.org/Protocols/rfc2616/rfc2616-sec6.html#sec6.1

[Elliot Murphy (statik)]

Martin, I agree with you about fixing the monitoring software as well and I am pursuing that in parallel. I set the urgency to high for the launchpad project simply as a matter of practical urgency - I want to be able to monitor the site right now, not wait for the monitoring software to get fixed, and was planning to deploy a local patch immediately in order to accomplish this, and then update whenever the change was made upstream. Dmitry responding quickly on behalf of Zope was a very pleasant surprise.

[Christian Theune (ctheune)]

Am Donnerstag, den 03.05.2007, 18:01 +0000 schrieb Martijn Pieters:
> Uhm, I know this is now fixed, but why was this set to a 'High'
> urgency?? The RFC states that the response code needs to recognized, but
> that the exact reason phrase wording is a recommendation only and can be
> localized. In other words, if I return HTTP/1.1 200 All is fine, Dude!
> I'd still be RFC compliant. The monitoring app is badly broken if it
> matches against the phrase!
>
> See http://www.w3.org/Protocols/rfc2616/rfc2616-sec6.html#sec6.1

Right. However, Zope's code was inconsistent at that place anyway!

And now we're even closer to the RFC. ;)

--
gocept gmbh & co. kg - forsterstraße 29 - 06112 halle/saale - germany
www.gocept.com - <email address hidden> - phone [phone] -
fax [phone] - zope and plone consulting and development

[Dmitry Vasiliev (hdima)]

Actually the fix makes the code cleaner and simpler so I think it's worth it in any case.

[Benji York (benji)]

This change has broken numerous tests in both Zope 3 and in projects that use Zope 3. Given that each value for "OK" are equally right and the net work involved in fixing the things this broke looks to be greater than reverting the change, this should be reverted.

[Dmitry Vasiliev (hdima)]

Hmm, I've fixed all tests on the trunk so I don't know that you mean. However I realize there are already many Zope 3 projects around so this fix can be too early for 3.4, but I think it's definitely should be fixed for 3.5. Is there already a Zope 3.4 branch? I'd prefer to revert the change on the branch only.

[Benji York (benji)]

Dmitry Vasiliev wrote:
> Hmm, I've fixed all tests on the trunk so I don't know that you mean.
>
zope.testbrowser was failing until I fixed it a week ago. The creation
of the satellite projects may have contributed to some being out of sync.

> However I realize there are already many Zope 3 projects around

Exactly.

> but I think it's definitely should be fixed for 3.5.

Why? The original code was not broken by any definition, and making the
change can only break existing code.
--
Benji York
Senior Software Engineer
Zope Corporation

[Christian Theune (ctheune)]

> Dmitry Vasiliev wrote:
>> Hmm, I've fixed all tests on the trunk so I don't know that you mean.
>>
> zope.testbrowser was failing until I fixed it a week ago. The creation
> of the satellite projects may have contributed to some being out of sync.
>
>> However I realize there are already many Zope 3 projects around
>
> Exactly.
>
>> but I think it's definitely should be fixed for 3.5.
>
> Why? The original code was not broken by any definition, and making the
> change can only break existing code.

It was not broken from the RFCs perspective. From an engineering
perspective I think it justified a refactoring to avoid code duplication.

[Benji York (benji)]

Christian Theune wrote:
>> Dmitry Vasiliev wrote:
>>> but I think it's definitely should be fixed for 3.5.
>> Why? The original code was not broken by any definition, and making the
>> change can only break existing code.
>
> It was not broken from the RFCs perspective. From an engineering
> perspective I think it justified a refactoring to avoid code duplication.

Refactoring changes the code without changing the output. That'd be
fine with me.

On the other hand, I still haven't heard any motivation to change the
reason phrase.
--
Benji York
Senior Software Engineer
Zope Corporation

[Dmitry Vasiliev (hdima)]

Benji York wrote:
> Dmitry Vasiliev wrote:
>> Hmm, I've fixed all tests on the trunk so I don't know that you mean.
>>
> zope.testbrowser was failing until I fixed it a week ago. The creation
> of the satellite projects may have contributed to some being out of sync.

Ah, changing the code which is now resided in different checkouts is a
pain and can be error prone.

>> but I think it's definitely should be fixed for 3.5.
>
> Why? The original code was not broken by any definition, and making the
> change can only break existing code.

I wanted to write about "Ok" as most used status text which is I believe
was true at some time in the past but by searching on the web I've found
mostly "OK" messages. So I'll change the message back. However in any
case we need to fix tests which depends on the status message.

And I realized that it's not so hard to customize the status message.
Now for customize the message per application you just need the
following code:

     # It wasn't work before the change
     zope.publisher.http.status_reasons[200] = "Ok"
     # It here just for clarity
     zope.publisher.http.init_status_codes()

And I guess you always can define IEndRequestEvent subscriber to
customize the message.

--
Dmitry Vasiliev <dima at hlabs.spb.ru>
http://hlabs.spb.ru

[Benji York (benji)]

Dmitry Vasiliev wrote:
> So I'll change the message back.

Thanks.

> And I realized that it's not so hard to customize the status message.

This would make a great FAQ entry. Even better: perhaps the wiki should
have a RecipeCategory for things like this.
--
Benji York
Senior Software Engineer
Zope Corporation

[Dmitry Vasiliev (hdima)]

Dmitry Vasiliev wrote:
> Benji York wrote:
>> Dmitry Vasiliev wrote:
>>> but I think it's definitely should be fixed for 3.5.
>> Why? The original code was not broken by any definition, and making the
>> change can only break existing code.
>
> I wanted to write about "Ok" as most used status text which is I believe
> was true at some time in the past but by searching on the web I've found
> mostly "OK" messages. So I'll change the message back. However in any
> case we need to fix tests which depends on the status message.

Ugh, it seems I need a few days of rest... :-( I've just looking at the
code and trying to understand why the publisher now returns '200 OK'
while yesterday I thought it should be '200 Ok' and then I've realized
that I've changed it from 'Ok' to 'OK' myself. The original request
propose to change the message from 'Ok' to 'OK' and I don't know why I
thought it was changed from 'OK' to 'Ok'. :-/

And now about motivation behind of this change. While RFC's doesn't
require Reason-Phrase for 200 was exactly 'OK' it's used in all examples
and it seems that 'OK' is de-facto standard which is also used by almost
all implementations including Apache, Zope 2 and Twisted.

Sorry for my mistake yesterday :-( but I think the message should be
'OK' by the reasons above.

--
Dmitry Vasiliev <dima at hlabs.spb.ru>
http://hlabs.spb.ru

[Benji York (benji)]

Dmitry Vasiliev wrote:
> And now about motivation behind of this change. While RFC's doesn't
> require Reason-Phrase for 200 was exactly 'OK' it's used in all examples
> and it seems that 'OK' is de-facto standard which is also used by almost
> all implementations including Apache, Zope 2 and Twisted.

That doesn't move me. The response phrase is informational only. A
human will understand "OK", "Ok", "ok", or "we're cool".

As such, the only thing accomplished by changing the response phrase is
breaking apps.
--
Benji York
Senior Software Engineer
Zope Corporation

[Dmitry Vasiliev (hdima)]

Benji York wrote:
> Dmitry Vasiliev wrote:
>> And I realized that it's not so hard to customize the status message.
>
> This would make a great FAQ entry. Even better: perhaps the wiki should
> have a RecipeCategory for things like this.

Great idea! There already was Zope 3 Cookbook project by Tarek which was
closed but was reincarnated by Baiju:
http://tarekziade.wordpress.com/2006/10/19/mini-book-a-dozen-recipes-for-zope-3/
It would be cool if the cookbook will be resided on the Zope wiki.

--
Dmitry Vasiliev <dima at hlabs.spb.ru>
http://hlabs.spb.ru

[Christian Theune (ctheune)]

Hi,

Am Montag, den 14.05.2007, 15:27 +0000 schrieb Benji York:
> Dmitry Vasiliev wrote:
> > And now about motivation behind of this change. While RFC's doesn't
> > require Reason-Phrase for 200 was exactly 'OK' it's used in all examples
> > and it seems that 'OK' is de-facto standard which is also used by almost
> > all implementations including Apache, Zope 2 and Twisted.
>
> That doesn't move me. The response phrase is informational only. A
> human will understand "OK", "Ok", "ok", or "we're cool".
>
> As such, the only thing accomplished by changing the response phrase is
> breaking apps.

Right. We tried to be nice here to one app that was broken to help the
launchpad guys. We didn't expect the RFC inquisition but of course got
bitten.

I agree to move back to the state before but I do propose to clean up
the special casing of 200 and use the lookup. We were never using the
"OK" from the dict so that entry should be updated to "Ok" as it's what
we were delivering before.

Assisting for one exception of a broken client broke other clients that
have the same right to be broken and the same amount of caringness.
However, the former status quo is probably the place to stay.

I'm with Benji.

Christian

--
gocept gmbh & co. kg - forsterstraße 29 - 06112 halle/saale - germany
www.gocept.com - <email address hidden> - phone [phone] -
fax [phone] - zope and plone consulting and development

[Christian Theune (ctheune)]

Am Montag, den 14.05.2007, 15:50 +0000 schrieb Dmitry Vasiliev:
> Benji York wrote:
> > Dmitry Vasiliev wrote:
> >> And I realized that it's not so hard to customize the status message.
> >
> > This would make a great FAQ entry. Even better: perhaps the wiki should
> > have a RecipeCategory for things like this.
>
> Great idea! There already was Zope 3 Cookbook project by Tarek which was
> closed but was reincarnated by Baiju:
> http://tarekziade.wordpress.com/2006/10/19/mini-book-a-dozen-recipes-for-zope-3/
> It would be cool if the cookbook will be resided on the Zope wiki.

Please move this part of the conversation to one of the developer
lists.

This thread is getting way too long for a bug tracker already.

--
gocept gmbh & co. kg - forsterstraße 29 - 06112 halle/saale - germany
www.gocept.com - <email address hidden> - phone [phone] -
fax [phone] - zope and plone consulting and development

[Dmitry Vasiliev (hdima)]

Benji York wrote:
> Dmitry Vasiliev wrote:
>> And now about motivation behind of this change. While RFC's doesn't
>> require Reason-Phrase for 200 was exactly 'OK' it's used in all examples
>> and it seems that 'OK' is de-facto standard which is also used by almost
>> all implementations including Apache, Zope 2 and Twisted.
>
> That doesn't move me. The response phrase is informational only. A
> human will understand "OK", "Ok", "ok", or "we're cool".

Yes and it can be customized as I've already mentioned before. But I
think by default the server should be as client-friendly as possible and
if we need to fix our apps for support some broken clients we definitely
need to do it.

> As such, the only thing accomplished by changing the response phrase is
> breaking apps.

I guess only tests are broken? If so they should be fixed anyway as long
as the message doesn't matter.

--
Dmitry Vasiliev <dima at hlabs.spb.ru>
http://hlabs.spb.ru

[Benji York (benji)]

Dmitry Vasiliev wrote:
> Yes and it can be customized as I've already mentioned before. But I
> think by default the server should be as client-friendly as possible and
> if we need to fix our apps for support some broken clients we definitely
> need to do it.

Between supporting a broken client and breaking current Zope 3 based
apps, I'll take the latter.

>> As such, the only thing accomplished by changing the response phrase is
>> breaking apps.
>
> I guess only tests are broken? If so they should be fixed anyway as long
> as the message doesn't matter.

No, as I said earlier I know of at least one Zope 3 app broken by this
change. As Christian said, breaking one set of apps to fix another
isn't something to be done lightly.
--
Benji York
Senior Software Engineer
Zope Corporation

[Dmitry Vasiliev (hdima)]

Benji York wrote:
> Dmitry Vasiliev wrote:
>>> As such, the only thing accomplished by changing the response phrase is
>>> breaking apps.
>> I guess only tests are broken? If so they should be fixed anyway as long
>> as the message doesn't matter.
>
> No, as I said earlier I know of at least one Zope 3 app broken by this
> change.

Ah, bad. :-(

> As Christian said, breaking one set of apps to fix another
> isn't something to be done lightly.

I agreed, I'll change the message back.

--
Dmitry Vasiliev <dima at hlabs.spb.ru>
http://hlabs.spb.ru

[Fred Drake (fdrake)]

On 5/15/07, Dmitry Vasiliev <email address hidden> wrote:
> I agreed, I'll change the message back.

Best would be to change the message back, and change tests to not care
about the message, only the 200 result code.

doctest.ELLIPSIS is a heavy hammer for this, unfortunately, though
commonly used.

  -Fred

--
Fred L. Drake, Jr. <fdrake at gmail.com>
"Chaos is the score upon which reality is written." --Henry Miller

[Dmitry Vasiliev (hdima)]

Fred Drake wrote:
> On 5/15/07, Dmitry Vasiliev <email address hidden> wrote:
>> I agreed, I'll change the message back.
>
> Best would be to change the message back, and change tests to not care
> about the message, only the 200 result code.
>
> doctest.ELLIPSIS is a heavy hammer for this, unfortunately, though
> commonly used.

Yes, first I want to fix tests external to zope.publisher with help of
ellipsis and then fix the message and test inside zope.publisher.
Without ellipsis I'll be bitten by the buildbot very often. :-)

--
Dmitry Vasiliev <dima at hlabs.spb.ru>
http://hlabs.spb.ru

[Jim Fulton (jim-zope)]

On May 15, 2007, at 10:07 AM, Fred Drake wrote:

> On 5/15/07, Dmitry Vasiliev <email address hidden> wrote:
>> I agreed, I'll change the message back.
>
> Best would be to change the message back, and change tests to not care
> about the message, only the 200 result code.
>
> doctest.ELLIPSIS is a heavy hammer for this, unfortunately, though
> commonly used.

zope.testing.renormalizing is far more appropriate.

Jim

--
Jim Fulton mailto:<email address hidden> Python Powered!
CTO [phone] http://www.python.org
Zope Corporation http://www.zope.com http://www.zope.org

[Benji York (benji)]

Fred Drake wrote:
> Best would be to change the message back, and change tests to not care
> about the message, only the 200 result code.
>
> doctest.ELLIPSIS is a heavy hammer for this, unfortunately, though
> commonly used.

Using the REnormalizer would yield nicer tests, that's what I did for
testbrowser.
--
Benji York
Senior Software Engineer
Zope Corporation

[Dmitry Vasiliev (hdima)]

The message reverted on the trunk at revision 75794

[Elliot Murphy (statik)]

per upstream discussion, this is not a bug.

[Martijn Pieters (mjpieters)]

As per discussion, the status for this bug should be 'Invalid'.