ZnClient receiving Expires header with Max-Age instead of HTTP formatted DateAndTime

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

ZnClient receiving Expires header with Max-Age instead of HTTP formatted DateAndTime

Paul DeBruicker
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
Reply | Threaded
Open this post in threaded view
|

Re: ZnClient receiving Expires header with Max-Age instead of HTTP formatted DateAndTime

Sven Van Caekenberghe-2
Hi Paul,

> On 11 Sep 2018, at 06:02, PAUL DEBRUICKER <[hidden email]> 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


Reply | Threaded
Open this post in threaded view
|

Re: ZnClient receiving Expires header with Max-Age instead of HTTP formatted DateAndTime

Erik Stel
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

Reply | Threaded
Open this post in threaded view
|

Re: ZnClient receiving Expires header with Max-Age instead of HTTP formatted DateAndTime

Sven Van Caekenberghe-2
In reply to this post by Sven Van Caekenberghe-2


> On 11 Sep 2018, at 13:53, Sven Van Caekenberghe <[hidden email]> wrote:
>
> Hi Paul,
>
>> On 11 Sep 2018, at 06:02, PAUL DEBRUICKER <[hidden email]> 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


Reply | Threaded
Open this post in threaded view
|

Re: ZnClient receiving Expires header with Max-Age instead of HTTP formatted DateAndTime

Sven Van Caekenberghe-2
In reply to this post by Erik Stel
Yes, that is how I read it now too.

> On 11 Sep 2018, at 14:55, Erik Stel <[hidden email]> 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
>


Reply | Threaded
Open this post in threaded view
|

Re: ZnClient receiving Expires header with Max-Age instead of HTTP formatted DateAndTime

Paul DeBruicker
In reply to this post by Sven Van Caekenberghe-2
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 &lt;

> pdebruic@

> &gt; 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

Reply | Threaded
Open this post in threaded view
|

Re: ZnClient receiving Expires header with Max-Age instead of HTTP formatted DateAndTime

Sven Van Caekenberghe-2


> On 11 Sep 2018, at 18:55, Paul DeBruicker <[hidden email]> 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 &lt;
>
>> pdebruic@
>
>> &gt; 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


Reply | Threaded
Open this post in threaded view
|

Re: ZnClient receiving Expires header with Max-Age instead of HTTP formatted DateAndTime

Paul DeBruicker
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 &lt;

> pdebruic@

> &gt; 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 &lt;
>>
>>> pdebruic@
>>
>>> &gt; 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