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 <[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 |
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 |
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 |
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 > |
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 < > 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 <[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 < > >> 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 |
Free forum by Nabble | Edit this page |