ZnClient receiving Expires header with Max-Age instead of HTTP formatted DateAndTime
Hi Sven - This is in Pharo 6.1. There is an API I'm using which sometimes returns a string only containing a single instance of the number 0 in the "Expires" field, so the #expiresTimeStamp method sends that to #parseHttpDate: and since it can't be parsed into a date an error is thrown. Seems like Zinc should be able to handle that based on the spec here: https://tools.ietf.org/html/rfc7234#section-5.3 Would you rather I changed the implementation of parseHttpDate: to add an empty check on the parsed tokens e.g. parseHttpDate: string "self parseHttpDate: 'Tue, 13 Sep 2011 08:04:49 GMT'." "self parseHttpDate: 'Tue, 13-Sep-2011 08:04:51 GMT'." "self parseHttpDate: 'Tue Jan 01 00:00:01 2036 GMT'." | tokens day month year hour minute second months map yearToken | string size = 1 ifTrue:[ ^DateAndTime epoch ]. tokens := (string findTokens: #( $ $- $: $, )) allButFirst. ... Or add a string size guard check in #expiresTimeStamp ? e.g. expiresTimeStamp self expires ifNil: [ ^ DateAndTime now + 1 day ] ifNotNil: [ :exp | exp size = 1 ifTrue: [ ^ DateAndTime epoch ]. ^ ZnUtils parseHttpDate: exp ] Thanks Paul
Hi Paul,
On 11 Sep 2018, at 06:02, PAUL DEBRUICKER <pdebruic@gmail.com> wrote:
Hi Sven -
This is in Pharo 6.1.
There is an API I'm using which sometimes returns a string only containing a single instance of the number 0 in the "Expires" field, so the #expiresTimeStamp method sends that to #parseHttpDate: and since it can't be parsed into a date an error is thrown. Seems like Zinc should be able to handle that based on the spec here: https://tools.ietf.org/html/rfc7234#section-5.3
Would you rather I changed the implementation of parseHttpDate: to add an empty check on the parsed tokens e.g.
IIRC some months ago, there were some changes to this area, but I forgot the details and the before state. Anyway, reading RFC7234's section that you reference, maybe we do need to make another change, but I am not sure. Let's start by explaining what there is today and why it is like that: #parseHttpDate: and #expiresTimestamp do indeed throw exceptions (by design), while #isExpired is the more high level access method. Consider: (ZnCookie name: 'foo' value: '100') isExpired. => false ZnUtils parseHttpDate: '0'. => Boom (ZnCookie name: 'foo' value: '100') expires: '0'; expiresTimeStamp. => Boom (ZnCookie name: 'foo' value: '100') expires: '0'; isExpired. => false Could you use #isExpired and does it do what you want/expect ? Reading the spec again, I am not 100% sure about the error case (existing but zero or wrong Expires header), maybe that should be true instead of false. isExpired | expirationTimeStamp | (self hasAttribute: 'expire') ifFalse: [ ^ false ]. [ expirationTimeStamp := self expiresTimeStamp ] on: Error do: [ ^ false ]. "note that max-age (#maxage) is not used" ^ expirationTimeStamp asUTC < DateAndTime now asUTC What do you think ? Anyone else with an opinion ? Sven
parseHttpDate: string "self parseHttpDate: 'Tue, 13 Sep 2011 08:04:49 GMT'." "self parseHttpDate: 'Tue, 13-Sep-2011 08:04:51 GMT'." "self parseHttpDate: 'Tue Jan 01 00:00:01 2036 GMT'."
| tokens day month year hour minute second months map yearToken | string size = 1 ifTrue:[ ^DateAndTime epoch ]. tokens := (string findTokens: #( $ $- $: $, )) allButFirst.
...
Or add a string size guard check in #expiresTimeStamp ? e.g.
expiresTimeStamp self expires ifNil: [ ^ DateAndTime now + 1 day ] ifNotNil: [ :exp | exp size = 1 ifTrue: [ ^ DateAndTime epoch ]. ^ ZnUtils parseHttpDate: exp ]
Thanks
Paul
Sven, According to the spec "0" or invalid date formats should mean 'isExpired' (as you already suggested): A cache recipient MUST interpret invalid date formats, especially the value "0", as representing a time in the past (i.e., "already expired"). So I would vote for having 'isExpired' answer 'true' in those cases. Having 'expiresTimeStamp' throw an error seems reasonable. The sender can catch the exception. Cheers, Erik -- Sent from: http://forum.world.st/Pharo-Smalltalk-Users-f1310670.html
Yes, that is how I read it now too.
On 11 Sep 2018, at 14:55, Erik Stel <erik.stel@gmail.com> wrote:
Sven,
According to the spec "0" or invalid date formats should mean 'isExpired' (as you already suggested): A cache recipient MUST interpret invalid date formats, especially the value "0", as representing a time in the past (i.e., "already expired").
So I would vote for having 'isExpired' answer 'true' in those cases. Having 'expiresTimeStamp' throw an error seems reasonable. The sender can catch the exception.
Cheers, Erik
-- Sent from: http://forum.world.st/Pharo-Smalltalk-Users-f1310670.html
On 11 Sep 2018, at 13:53, Sven Van Caekenberghe <sven@stfx.eu> wrote:
Hi Paul,
On 11 Sep 2018, at 06:02, PAUL DEBRUICKER <pdebruic@gmail.com> wrote:
Hi Sven -
This is in Pharo 6.1.
There is an API I'm using which sometimes returns a string only containing a single instance of the number 0 in the "Expires" field, so the #expiresTimeStamp method sends that to #parseHttpDate: and since it can't be parsed into a date an error is thrown. Seems like Zinc should be able to handle that based on the spec here: https://tools.ietf.org/html/rfc7234#section-5.3
Would you rather I changed the implementation of parseHttpDate: to add an empty check on the parsed tokens e.g.
IIRC some months ago, there were some changes to this area, but I forgot the details and the before state.
Anyway, reading RFC7234's section that you reference, maybe we do need to make another change, but I am not sure.
Let's start by explaining what there is today and why it is like that: #parseHttpDate: and #expiresTimestamp do indeed throw exceptions (by design), while #isExpired is the more high level access method.
Consider:
(ZnCookie name: 'foo' value: '100') isExpired.
=> false
ZnUtils parseHttpDate: '0'.
=> Boom
(ZnCookie name: 'foo' value: '100') expires: '0'; expiresTimeStamp.
=> Boom
(ZnCookie name: 'foo' value: '100') expires: '0'; isExpired.
=> false
Could you use #isExpired and does it do what you want/expect ?
Reading the spec again, I am not 100% sure about the error case (existing but zero or wrong Expires header), maybe that should be true instead of false.
isExpired | expirationTimeStamp | (self hasAttribute: 'expire') ifFalse: [ ^ false ]. [ expirationTimeStamp := self expiresTimeStamp ] on: Error do: [ ^ false ]. "note that max-age (#maxage) is not used" ^ expirationTimeStamp asUTC < DateAndTime now asUTC
What do you think ? Anyone else with an opinion ?
I just noticed that we are mixing request/response http headers with cookies, which might not be a good idea.
Sven
parseHttpDate: string "self parseHttpDate: 'Tue, 13 Sep 2011 08:04:49 GMT'." "self parseHttpDate: 'Tue, 13-Sep-2011 08:04:51 GMT'." "self parseHttpDate: 'Tue Jan 01 00:00:01 2036 GMT'."
| tokens day month year hour minute second months map yearToken | string size = 1 ifTrue:[ ^DateAndTime epoch ]. tokens := (string findTokens: #( $ $- $: $, )) allButFirst.
...
Or add a string size guard check in #expiresTimeStamp ? e.g.
expiresTimeStamp self expires ifNil: [ ^ DateAndTime now + 1 day ] ifNotNil: [ :exp | exp size = 1 ifTrue: [ ^ DateAndTime epoch ]. ^ ZnUtils parseHttpDate: exp ]
Thanks
Paul
Seems like the invalid date could be anything and the spec gives '0' as the example invalid date. I'm not using #expiresTimeStamp or #parseHttpDate: explicitly its just that the GET request was failing on that date parsing error, when checking if the Cookie was expired. I think your proposed ZnCookie>>#isExpired is the right way to go e.g. isExpired | expirationTimeStamp | (self hasAttribute: 'expire') ifFalse: [ ^ false ]. [ expirationTimeStamp := self expiresTimeStamp ] on: Error do: [ ^ false ]. "note that max-age (#maxage) is not used" ^ expirationTimeStamp asUTC < DateAndTime now asUTC Sven Van Caekenberghe-2 wrote
Hi Paul,
On 11 Sep 2018, at 06:02, PAUL DEBRUICKER <
pdebruic@
> wrote:
Hi Sven -
This is in Pharo 6.1.
There is an API I'm using which sometimes returns a string only containing a single instance of the number 0 in the "Expires" field, so the #expiresTimeStamp method sends that to #parseHttpDate: and since it can't be parsed into a date an error is thrown. Seems like Zinc should be able to handle that based on the spec here: https://tools.ietf.org/html/rfc7234#section-5.3
Would you rather I changed the implementation of parseHttpDate: to add an empty check on the parsed tokens e.g.
IIRC some months ago, there were some changes to this area, but I forgot the details and the before state.
Anyway, reading RFC7234's section that you reference, maybe we do need to make another change, but I am not sure.
Let's start by explaining what there is today and why it is like that: #parseHttpDate: and #expiresTimestamp do indeed throw exceptions (by design), while #isExpired is the more high level access method.
Consider:
(ZnCookie name: 'foo' value: '100') isExpired.
=> false
ZnUtils parseHttpDate: '0'.
=> Boom
(ZnCookie name: 'foo' value: '100') expires: '0'; expiresTimeStamp.
=> Boom
(ZnCookie name: 'foo' value: '100') expires: '0'; isExpired.
=> false
Could you use #isExpired and does it do what you want/expect ?
Reading the spec again, I am not 100% sure about the error case (existing but zero or wrong Expires header), maybe that should be true instead of false.
isExpired | expirationTimeStamp | (self hasAttribute: 'expire') ifFalse: [ ^ false ]. [ expirationTimeStamp := self expiresTimeStamp ] on: Error do: [ ^ false ]. "note that max-age (#maxage) is not used" ^ expirationTimeStamp asUTC < DateAndTime now asUTC
What do you think ? Anyone else with an opinion ?
Sven
parseHttpDate: string "self parseHttpDate: 'Tue, 13 Sep 2011 08:04:49 GMT'." "self parseHttpDate: 'Tue, 13-Sep-2011 08:04:51 GMT'." "self parseHttpDate: 'Tue Jan 01 00:00:01 2036 GMT'."
| tokens day month year hour minute second months map yearToken | string size = 1 ifTrue:[ ^DateAndTime epoch ]. tokens := (string findTokens: #( $ $- $: $, )) allButFirst.
...
Or add a string size guard check in #expiresTimeStamp ? e.g.
expiresTimeStamp self expires ifNil: [ ^ DateAndTime now + 1 day ] ifNotNil: [ :exp | exp size = 1 ifTrue: [ ^ DateAndTime epoch ]. ^ ZnUtils parseHttpDate: exp ]
Thanks
Paul
-- Sent from: http://forum.world.st/Pharo-Smalltalk-Users-f1310670.html
On 11 Sep 2018, at 18:55, Paul DeBruicker <pdebruic@gmail.com> wrote:
Seems like the invalid date could be anything and the spec gives '0' as the example invalid date.
I'm not using #expiresTimeStamp or #parseHttpDate: explicitly its just that the GET request was failing on that date parsing error, when checking if the Cookie was expired.
I think your proposed ZnCookie>>#isExpired is the right way to go e.g.
OK, but that is already in the code. I am looking at Zinc-HTTP-SvenVanCaekenberghe.475 ... is that not what you are seeing ?
isExpired | expirationTimeStamp | (self hasAttribute: 'expire') ifFalse: [ ^ false ]. [ expirationTimeStamp := self expiresTimeStamp ] on: Error do: [ ^ false ]. "note that max-age (#maxage) is not used" ^ expirationTimeStamp asUTC < DateAndTime now asUTC
Sven Van Caekenberghe-2 wrote
Hi Paul,
On 11 Sep 2018, at 06:02, PAUL DEBRUICKER <
pdebruic@
> wrote:
Hi Sven -
This is in Pharo 6.1.
There is an API I'm using which sometimes returns a string only containing a single instance of the number 0 in the "Expires" field, so the #expiresTimeStamp method sends that to #parseHttpDate: and since it can't be parsed into a date an error is thrown. Seems like Zinc should be able to handle that based on the spec here: https://tools.ietf.org/html/rfc7234#section-5.3
Would you rather I changed the implementation of parseHttpDate: to add an empty check on the parsed tokens e.g.
IIRC some months ago, there were some changes to this area, but I forgot the details and the before state.
Anyway, reading RFC7234's section that you reference, maybe we do need to make another change, but I am not sure.
Let's start by explaining what there is today and why it is like that: #parseHttpDate: and #expiresTimestamp do indeed throw exceptions (by design), while #isExpired is the more high level access method.
Consider:
(ZnCookie name: 'foo' value: '100') isExpired.
=> false
ZnUtils parseHttpDate: '0'.
=> Boom
(ZnCookie name: 'foo' value: '100') expires: '0'; expiresTimeStamp.
=> Boom
(ZnCookie name: 'foo' value: '100') expires: '0'; isExpired.
=> false
Could you use #isExpired and does it do what you want/expect ?
Reading the spec again, I am not 100% sure about the error case (existing but zero or wrong Expires header), maybe that should be true instead of false.
isExpired | expirationTimeStamp | (self hasAttribute: 'expire') ifFalse: [ ^ false ]. [ expirationTimeStamp := self expiresTimeStamp ] on: Error do: [ ^ false ]. "note that max-age (#maxage) is not used" ^ expirationTimeStamp asUTC < DateAndTime now asUTC
What do you think ? Anyone else with an opinion ?
Sven
parseHttpDate: string "self parseHttpDate: 'Tue, 13 Sep 2011 08:04:49 GMT'." "self parseHttpDate: 'Tue, 13-Sep-2011 08:04:51 GMT'." "self parseHttpDate: 'Tue Jan 01 00:00:01 2036 GMT'."
| tokens day month year hour minute second months map yearToken | string size = 1 ifTrue:[ ^DateAndTime epoch ]. tokens := (string findTokens: #( $ $- $: $, )) allButFirst.
...
Or add a string size guard check in #expiresTimeStamp ? e.g.
expiresTimeStamp self expires ifNil: [ ^ DateAndTime now + 1 day ] ifNotNil: [ :exp | exp size = 1 ifTrue: [ ^ DateAndTime epoch ]. ^ ZnUtils parseHttpDate: exp ]
Thanks
Paul
-- Sent from: http://forum.world.st/Pharo-Smalltalk-Users-f1310670.html
No. I thought I was using Pharo 6.1 but in the "About..." menu item it shows: Pharo 6.0 Latest update: #60540 The ZincHTTPComponents version in that image is TheIntegrator.461 Anyway. I'm glad its fixed in the more recent version. Apologies for the noise. Sven Van Caekenberghe-2 wrote
On 11 Sep 2018, at 18:55, Paul DeBruicker <
pdebruic@
> wrote:
Seems like the invalid date could be anything and the spec gives '0' as the example invalid date.
I'm not using #expiresTimeStamp or #parseHttpDate: explicitly its just that the GET request was failing on that date parsing error, when checking if the Cookie was expired.
I think your proposed ZnCookie>>#isExpired is the right way to go e.g.
OK, but that is already in the code. I am looking at Zinc-HTTP-SvenVanCaekenberghe.475 ... is that not what you are seeing ?
isExpired | expirationTimeStamp | (self hasAttribute: 'expire') ifFalse: [ ^ false ]. [ expirationTimeStamp := self expiresTimeStamp ] on: Error do: [ ^ false ]. "note that max-age (#maxage) is not used" ^ expirationTimeStamp asUTC < DateAndTime now asUTC
Sven Van Caekenberghe-2 wrote
Hi Paul,
On 11 Sep 2018, at 06:02, PAUL DEBRUICKER <
pdebruic@
> wrote:
Hi Sven -
This is in Pharo 6.1.
There is an API I'm using which sometimes returns a string only containing a single instance of the number 0 in the "Expires" field, so the #expiresTimeStamp method sends that to #parseHttpDate: and since it can't be parsed into a date an error is thrown. Seems like Zinc should be able to handle that based on the spec here: https://tools.ietf.org/html/rfc7234#section-5.3
Would you rather I changed the implementation of parseHttpDate: to add an empty check on the parsed tokens e.g.
IIRC some months ago, there were some changes to this area, but I forgot the details and the before state.
Anyway, reading RFC7234's section that you reference, maybe we do need to make another change, but I am not sure.
Let's start by explaining what there is today and why it is like that: #parseHttpDate: and #expiresTimestamp do indeed throw exceptions (by design), while #isExpired is the more high level access method.
Consider:
(ZnCookie name: 'foo' value: '100') isExpired.
=> false
ZnUtils parseHttpDate: '0'.
=> Boom
(ZnCookie name: 'foo' value: '100') expires: '0'; expiresTimeStamp.
=> Boom
(ZnCookie name: 'foo' value: '100') expires: '0'; isExpired.
=> false
Could you use #isExpired and does it do what you want/expect ?
Reading the spec again, I am not 100% sure about the error case (existing but zero or wrong Expires header), maybe that should be true instead of false.
isExpired | expirationTimeStamp | (self hasAttribute: 'expire') ifFalse: [ ^ false ]. [ expirationTimeStamp := self expiresTimeStamp ] on: Error do: [ ^ false ]. "note that max-age (#maxage) is not used" ^ expirationTimeStamp asUTC < DateAndTime now asUTC
What do you think ? Anyone else with an opinion ?
Sven
parseHttpDate: string "self parseHttpDate: 'Tue, 13 Sep 2011 08:04:49 GMT'." "self parseHttpDate: 'Tue, 13-Sep-2011 08:04:51 GMT'." "self parseHttpDate: 'Tue Jan 01 00:00:01 2036 GMT'."
| tokens day month year hour minute second months map yearToken | string size = 1 ifTrue:[ ^DateAndTime epoch ]. tokens := (string findTokens: #( $ $- $: $, )) allButFirst.
...
Or add a string size guard check in #expiresTimeStamp ? e.g.
expiresTimeStamp self expires ifNil: [ ^ DateAndTime now + 1 day ] ifNotNil: [ :exp | exp size = 1 ifTrue: [ ^ DateAndTime epoch ]. ^ ZnUtils parseHttpDate: exp ]
Thanks
Paul
-- Sent from: http://forum.world.st/Pharo-Smalltalk-Users-f1310670.html
-- Sent from: http://forum.world.st/Pharo-Smalltalk-Users-f1310670.html
participants (4)
-
Erik Stel -
PAUL DEBRUICKER -
Paul DeBruicker
-
Sven Van Caekenberghe