Issue with ZnMimeType>>#=
Hi, Yesterday, I stumbled across a bug when using the Zinc Seaside adaptor that resulted in that the post fields of a request were not present in the WARequest. I found out that the problem occurred when converting a Zinc request into a WARequest. More particular: ZnZincServerAdaptor>>requestFieldsFor: aZincRequest | fields | fields := WARequestFields new. (aZincRequest method = #POST and: [ aZincRequest hasEntity ]) ifTrue: [ aZincRequest entity contentType = ZnMimeType applicationFormUrlEncoded ifTrue: [ fields addAll: aZincRequest entity fields ]. aZincRequest entity contentType = ZnMimeType multiPartFormData ifTrue: [ fields addAll: (self convertMultipart: aZincRequest entity) ] ]. ^ fields This method checks if the Mime Type is either applicationFormUrlEncoded or multiPartFormData in order to extract the post fields. Recently, ZnMimeType>>= got changed from: ZnMimeType>>= other ^ (self class == other class) and: [ self main = other main and: [ self sub = other sub ] ] to: ZnMimeType>>= other ^ (self class == other class) and: [ self main = other main and: [ self sub = other sub and: [ self hasParameters not & other hasParameters not or: [ self parameters = other parameters ] ] ] ] As my request had an additional parameter charset=utf-8, the equality with ZnMimeType applicationFormUrlEncoded failed and hence the fields were not extract. While I assume that the comparison of the Mime type parameters in #= is there for a good reason, and the best way to fix this problem is to change the #requestFieldsFor: method in order to match the Mime type differently, I wanted to briefly check with the mailing list before opening an issue + submitting a patch. Kind regards, Andy
Hi Andy, Yes, I forgot about ZnZincServerAdaptor when refactoring/changing ZnMimeType>>#= - sorry about that. The solution is actually quite easy: ZnMimeType>>#match: does a comparison while ignoring additional mime type parameters like charset, in addition to its wild card matching. So the patch then would be: ZnZincServerAdaptor>>requestFieldsFor: aZincRequest | fields | fields := WARequestFields new. (aZincRequest method = #POST and: [ aZincRequest hasEntity ]) ifTrue: [ (aZincRequest entity contentType match: ZnMimeType applicationFormUrlEncoded) ifTrue: [ fields addAll: aZincRequest entity fields ]. (aZincRequest entity contentType match: ZnMimeType multiPartFormData) ifTrue: [ fields addAll: (self convertMultipart: aZincRequest entity) ] ]. ^ fields If you confirm that this works, I will check in the change later today. Thanks again for the cristal clear bug report ! Regards, Sven On 04 Jun 2013, at 10:34, Andy Kellens <akellens@vub.ac.be> wrote:
Hi,
Yesterday, I stumbled across a bug when using the Zinc Seaside adaptor that resulted in that the post fields of a request were not present in the WARequest. I found out that the problem occurred when converting a Zinc request into a WARequest.
More particular: ZnZincServerAdaptor>>requestFieldsFor: aZincRequest | fields | fields := WARequestFields new. (aZincRequest method = #POST and: [ aZincRequest hasEntity ]) ifTrue: [ aZincRequest entity contentType = ZnMimeType applicationFormUrlEncoded ifTrue: [ fields addAll: aZincRequest entity fields ]. aZincRequest entity contentType = ZnMimeType multiPartFormData ifTrue: [ fields addAll: (self convertMultipart: aZincRequest entity) ] ]. ^ fields
This method checks if the Mime Type is either applicationFormUrlEncoded or multiPartFormData in order to extract the post fields.
Recently, ZnMimeType>>= got changed from: ZnMimeType>>= other ^ (self class == other class) and: [ self main = other main and: [ self sub = other sub ] ]
to: ZnMimeType>>= other ^ (self class == other class) and: [ self main = other main and: [ self sub = other sub and: [ self hasParameters not & other hasParameters not or: [ self parameters = other parameters ] ] ] ]
As my request had an additional parameter charset=utf-8, the equality with ZnMimeType applicationFormUrlEncoded failed and hence the fields were not extract.
While I assume that the comparison of the Mime type parameters in #= is there for a good reason, and the best way to fix this problem is to change the #requestFieldsFor: method in order to match the Mime type differently, I wanted to briefly check with the mailing list before opening an issue + submitting a patch.
Kind regards,
Andy
-- Sven Van Caekenberghe Proudly supporting Pharo http://pharo.org http://association.pharo.org http://consortium.pharo.org
Hi Sven, Thanks for the quick response! A slightly altered version of your patch (it is #matches: instead of #match:) results in that the code goes into the right branch. ZnZincServerAdaptor>>requestFieldsFor: aZincRequest | fields | fields := WARequestFields new. (aZincRequest method = #POST and: [ aZincRequest hasEntity ]) ifTrue: [ (aZincRequest entity contentType matches: ZnMimeType applicationFormUrlEncoded) ifTrue: [ fields addAll: aZincRequest entity fields ]. (aZincRequest entity contentType matches: ZnMimeType multiPartFormData) ifTrue: [ fields addAll: (self convertMultipart: aZincRequest entity) ] ]. ^ fields Now, to fix my problem I still had to patch two other methods to also use #matches: instead of #= ZnEntity>>contentType: object "We only allow assignment compatible with our designated mime type" | newType | newType := object asZnMimeType. (contentType notNil and:[contentType matches: newType]) ifTrue: [ ^ self ] ifFalse: [ (self class designatedMimeType isNil or: [ self class designatedMimeType matches: newType ]) ifTrue: [ contentType := newType ] ] and: ZnEntity class>>concreteSubclassForType: mimeType binary: forceBinary "Answer the concrete ZnEntity subclass that handles mimeType" ^ self allSubclasses detect: [ :each | each designatedMimeType matches: mimeType ] ifNone: [ (mimeType isBinary or: [ forceBinary ]) ifTrue: [ self byteArrayEntityClass ] ifFalse: [ self stringEntityClass ] ] With these changes, everything works like a charm again. Kind regards, Andy On 04 Jun 2013, at 10:44, Sven Van Caekenberghe <sven@stfx.eu> wrote:
Hi Andy,
Yes, I forgot about ZnZincServerAdaptor when refactoring/changing ZnMimeType>>#= - sorry about that. The solution is actually quite easy: ZnMimeType>>#match: does a comparison while ignoring additional mime type parameters like charset, in addition to its wild card matching.
So the patch then would be:
ZnZincServerAdaptor>>requestFieldsFor: aZincRequest | fields | fields := WARequestFields new. (aZincRequest method = #POST and: [ aZincRequest hasEntity ]) ifTrue: [ (aZincRequest entity contentType match: ZnMimeType applicationFormUrlEncoded) ifTrue: [ fields addAll: aZincRequest entity fields ]. (aZincRequest entity contentType match: ZnMimeType multiPartFormData) ifTrue: [ fields addAll: (self convertMultipart: aZincRequest entity) ] ]. ^ fields
If you confirm that this works, I will check in the change later today.
Thanks again for the cristal clear bug report !
Regards,
Sven
On 04 Jun 2013, at 10:34, Andy Kellens <akellens@vub.ac.be> wrote:
Hi,
Yesterday, I stumbled across a bug when using the Zinc Seaside adaptor that resulted in that the post fields of a request were not present in the WARequest. I found out that the problem occurred when converting a Zinc request into a WARequest.
More particular: ZnZincServerAdaptor>>requestFieldsFor: aZincRequest | fields | fields := WARequestFields new. (aZincRequest method = #POST and: [ aZincRequest hasEntity ]) ifTrue: [ aZincRequest entity contentType = ZnMimeType applicationFormUrlEncoded ifTrue: [ fields addAll: aZincRequest entity fields ]. aZincRequest entity contentType = ZnMimeType multiPartFormData ifTrue: [ fields addAll: (self convertMultipart: aZincRequest entity) ] ]. ^ fields
This method checks if the Mime Type is either applicationFormUrlEncoded or multiPartFormData in order to extract the post fields.
Recently, ZnMimeType>>= got changed from: ZnMimeType>>= other ^ (self class == other class) and: [ self main = other main and: [ self sub = other sub ] ]
to: ZnMimeType>>= other ^ (self class == other class) and: [ self main = other main and: [ self sub = other sub and: [ self hasParameters not & other hasParameters not or: [ self parameters = other parameters ] ] ] ]
As my request had an additional parameter charset=utf-8, the equality with ZnMimeType applicationFormUrlEncoded failed and hence the fields were not extract.
While I assume that the comparison of the Mime type parameters in #= is there for a good reason, and the best way to fix this problem is to change the #requestFieldsFor: method in order to match the Mime type differently, I wanted to briefly check with the mailing list before opening an issue + submitting a patch.
Kind regards,
Andy
-- Sven Van Caekenberghe Proudly supporting Pharo http://pharo.org http://association.pharo.org http://consortium.pharo.org
On 04 Jun 2013, at 11:04, Andy Kellens <akellens@vub.ac.be> wrote:
Hi Sven,
Thanks for the quick response!
A slightly altered version of your patch (it is #matches: instead of #match:) results in that the code goes into the right branch.
ZnZincServerAdaptor>>requestFieldsFor: aZincRequest | fields | fields := WARequestFields new. (aZincRequest method = #POST and: [ aZincRequest hasEntity ]) ifTrue: [ (aZincRequest entity contentType matches: ZnMimeType applicationFormUrlEncoded) ifTrue: [ fields addAll: aZincRequest entity fields ]. (aZincRequest entity contentType matches: ZnMimeType multiPartFormData) ifTrue: [ fields addAll: (self convertMultipart: aZincRequest entity) ] ]. ^ fields
Yes, sorry for the typo.
Now, to fix my problem I still had to patch two other methods to also use #matches: instead of #= ZnEntity>>contentType: object "We only allow assignment compatible with our designated mime type"
| newType | newType := object asZnMimeType. (contentType notNil and:[contentType matches: newType]) ifTrue: [ ^ self ] ifFalse: [ (self class designatedMimeType isNil or: [ self class designatedMimeType matches: newType ]) ifTrue: [ contentType := newType ] ]
That makes sense, but I would say that the first condition should still use #= otherwise it would no longer be possible to change the mime type from text/plain to text/plain;charset=ascii for example (they types then match and nothing will be done). If you make that change, does it still work for you ?
and: ZnEntity class>>concreteSubclassForType: mimeType binary: forceBinary "Answer the concrete ZnEntity subclass that handles mimeType"
^ self allSubclasses detect: [ :each | each designatedMimeType matches: mimeType ] ifNone: [ (mimeType isBinary or: [ forceBinary ]) ifTrue: [ self byteArrayEntityClass ] ifFalse: [ self stringEntityClass ] ]
That one makes sense indeed.
With these changes, everything works like a charm again.
Kind regards,
Andy
On 04 Jun 2013, at 10:44, Sven Van Caekenberghe <sven@stfx.eu> wrote:
Hi Andy,
Yes, I forgot about ZnZincServerAdaptor when refactoring/changing ZnMimeType>>#= - sorry about that. The solution is actually quite easy: ZnMimeType>>#match: does a comparison while ignoring additional mime type parameters like charset, in addition to its wild card matching.
So the patch then would be:
ZnZincServerAdaptor>>requestFieldsFor: aZincRequest | fields | fields := WARequestFields new. (aZincRequest method = #POST and: [ aZincRequest hasEntity ]) ifTrue: [ (aZincRequest entity contentType match: ZnMimeType applicationFormUrlEncoded) ifTrue: [ fields addAll: aZincRequest entity fields ]. (aZincRequest entity contentType match: ZnMimeType multiPartFormData) ifTrue: [ fields addAll: (self convertMultipart: aZincRequest entity) ] ]. ^ fields
If you confirm that this works, I will check in the change later today.
Thanks again for the cristal clear bug report !
Regards,
Sven
On 04 Jun 2013, at 10:34, Andy Kellens <akellens@vub.ac.be> wrote:
Hi,
Yesterday, I stumbled across a bug when using the Zinc Seaside adaptor that resulted in that the post fields of a request were not present in the WARequest. I found out that the problem occurred when converting a Zinc request into a WARequest.
More particular: ZnZincServerAdaptor>>requestFieldsFor: aZincRequest | fields | fields := WARequestFields new. (aZincRequest method = #POST and: [ aZincRequest hasEntity ]) ifTrue: [ aZincRequest entity contentType = ZnMimeType applicationFormUrlEncoded ifTrue: [ fields addAll: aZincRequest entity fields ]. aZincRequest entity contentType = ZnMimeType multiPartFormData ifTrue: [ fields addAll: (self convertMultipart: aZincRequest entity) ] ]. ^ fields
This method checks if the Mime Type is either applicationFormUrlEncoded or multiPartFormData in order to extract the post fields.
Recently, ZnMimeType>>= got changed from: ZnMimeType>>= other ^ (self class == other class) and: [ self main = other main and: [ self sub = other sub ] ]
to: ZnMimeType>>= other ^ (self class == other class) and: [ self main = other main and: [ self sub = other sub and: [ self hasParameters not & other hasParameters not or: [ self parameters = other parameters ] ] ] ]
As my request had an additional parameter charset=utf-8, the equality with ZnMimeType applicationFormUrlEncoded failed and hence the fields were not extract.
While I assume that the comparison of the Mime type parameters in #= is there for a good reason, and the best way to fix this problem is to change the #requestFieldsFor: method in order to match the Mime type differently, I wanted to briefly check with the mailing list before opening an issue + submitting a patch.
Kind regards,
Andy
-- Sven Van Caekenberghe Proudly supporting Pharo http://pharo.org http://association.pharo.org http://consortium.pharo.org
On 04 Jun 2013, at 11:13, Sven Van Caekenberghe <sven@stfx.eu> wrote:
On 04 Jun 2013, at 11:04, Andy Kellens <akellens@vub.ac.be> wrote:
Hi Sven,
Thanks for the quick response!
A slightly altered version of your patch (it is #matches: instead of #match:) results in that the code goes into the right branch.
ZnZincServerAdaptor>>requestFieldsFor: aZincRequest | fields | fields := WARequestFields new. (aZincRequest method = #POST and: [ aZincRequest hasEntity ]) ifTrue: [ (aZincRequest entity contentType matches: ZnMimeType applicationFormUrlEncoded) ifTrue: [ fields addAll: aZincRequest entity fields ]. (aZincRequest entity contentType matches: ZnMimeType multiPartFormData) ifTrue: [ fields addAll: (self convertMultipart: aZincRequest entity) ] ]. ^ fields
Yes, sorry for the typo.
Now, to fix my problem I still had to patch two other methods to also use #matches: instead of #= ZnEntity>>contentType: object "We only allow assignment compatible with our designated mime type"
| newType | newType := object asZnMimeType. (contentType notNil and:[contentType matches: newType]) ifTrue: [ ^ self ] ifFalse: [ (self class designatedMimeType isNil or: [ self class designatedMimeType matches: newType ]) ifTrue: [ contentType := newType ] ]
That makes sense, but I would say that the first condition should still use #= otherwise it would no longer be possible to change the mime type from text/plain to text/plain;charset=ascii for example (they types then match and nothing will be done). If you make that change, does it still work for you ?
If I change it back to #=, the entity of the request is no longer a ZnApplicationFormUrlEncodedEntity but a ZnByteArrayEntity. When retrieving the fields, I get a #DNU that ZnByteArrayEntity does not understand #fields.
and: ZnEntity class>>concreteSubclassForType: mimeType binary: forceBinary "Answer the concrete ZnEntity subclass that handles mimeType"
^ self allSubclasses detect: [ :each | each designatedMimeType matches: mimeType ] ifNone: [ (mimeType isBinary or: [ forceBinary ]) ifTrue: [ self byteArrayEntityClass ] ifFalse: [ self stringEntityClass ] ]
That one makes sense indeed.
With these changes, everything works like a charm again.
Kind regards,
Andy
On 04 Jun 2013, at 10:44, Sven Van Caekenberghe <sven@stfx.eu> wrote:
Hi Andy,
Yes, I forgot about ZnZincServerAdaptor when refactoring/changing ZnMimeType>>#= - sorry about that. The solution is actually quite easy: ZnMimeType>>#match: does a comparison while ignoring additional mime type parameters like charset, in addition to its wild card matching.
So the patch then would be:
ZnZincServerAdaptor>>requestFieldsFor: aZincRequest | fields | fields := WARequestFields new. (aZincRequest method = #POST and: [ aZincRequest hasEntity ]) ifTrue: [ (aZincRequest entity contentType match: ZnMimeType applicationFormUrlEncoded) ifTrue: [ fields addAll: aZincRequest entity fields ]. (aZincRequest entity contentType match: ZnMimeType multiPartFormData) ifTrue: [ fields addAll: (self convertMultipart: aZincRequest entity) ] ]. ^ fields
If you confirm that this works, I will check in the change later today.
Thanks again for the cristal clear bug report !
Regards,
Sven
On 04 Jun 2013, at 10:34, Andy Kellens <akellens@vub.ac.be> wrote:
Hi,
Yesterday, I stumbled across a bug when using the Zinc Seaside adaptor that resulted in that the post fields of a request were not present in the WARequest. I found out that the problem occurred when converting a Zinc request into a WARequest.
More particular: ZnZincServerAdaptor>>requestFieldsFor: aZincRequest | fields | fields := WARequestFields new. (aZincRequest method = #POST and: [ aZincRequest hasEntity ]) ifTrue: [ aZincRequest entity contentType = ZnMimeType applicationFormUrlEncoded ifTrue: [ fields addAll: aZincRequest entity fields ]. aZincRequest entity contentType = ZnMimeType multiPartFormData ifTrue: [ fields addAll: (self convertMultipart: aZincRequest entity) ] ]. ^ fields
This method checks if the Mime Type is either applicationFormUrlEncoded or multiPartFormData in order to extract the post fields.
Recently, ZnMimeType>>= got changed from: ZnMimeType>>= other ^ (self class == other class) and: [ self main = other main and: [ self sub = other sub ] ]
to: ZnMimeType>>= other ^ (self class == other class) and: [ self main = other main and: [ self sub = other sub and: [ self hasParameters not & other hasParameters not or: [ self parameters = other parameters ] ] ] ]
As my request had an additional parameter charset=utf-8, the equality with ZnMimeType applicationFormUrlEncoded failed and hence the fields were not extract.
While I assume that the comparison of the Mime type parameters in #= is there for a good reason, and the best way to fix this problem is to change the #requestFieldsFor: method in order to match the Mime type differently, I wanted to briefly check with the mailing list before opening an issue + submitting a patch.
Kind regards,
Andy
-- Sven Van Caekenberghe Proudly supporting Pharo http://pharo.org http://association.pharo.org http://consortium.pharo.org
On 04 Jun 2013, at 11:20, Andy Kellens <akellens@vub.ac.be> wrote:
On 04 Jun 2013, at 11:13, Sven Van Caekenberghe <sven@stfx.eu> wrote:
On 04 Jun 2013, at 11:04, Andy Kellens <akellens@vub.ac.be> wrote:
Hi Sven,
Thanks for the quick response!
A slightly altered version of your patch (it is #matches: instead of #match:) results in that the code goes into the right branch.
ZnZincServerAdaptor>>requestFieldsFor: aZincRequest | fields | fields := WARequestFields new. (aZincRequest method = #POST and: [ aZincRequest hasEntity ]) ifTrue: [ (aZincRequest entity contentType matches: ZnMimeType applicationFormUrlEncoded) ifTrue: [ fields addAll: aZincRequest entity fields ]. (aZincRequest entity contentType matches: ZnMimeType multiPartFormData) ifTrue: [ fields addAll: (self convertMultipart: aZincRequest entity) ] ]. ^ fields
Yes, sorry for the typo.
Now, to fix my problem I still had to patch two other methods to also use #matches: instead of #= ZnEntity>>contentType: object "We only allow assignment compatible with our designated mime type"
| newType | newType := object asZnMimeType. (contentType notNil and:[contentType matches: newType]) ifTrue: [ ^ self ] ifFalse: [ (self class designatedMimeType isNil or: [ self class designatedMimeType matches: newType ]) ifTrue: [ contentType := newType ] ]
That makes sense, but I would say that the first condition should still use #= otherwise it would no longer be possible to change the mime type from text/plain to text/plain;charset=ascii for example (they types then match and nothing will be done). If you make that change, does it still work for you ?
If I change it back to #=, the entity of the request is no longer a ZnApplicationFormUrlEncodedEntity but a ZnByteArrayEntity. When retrieving the fields, I get a #DNU that ZnByteArrayEntity does not understand #fields.
I will go into a Seaside image myself later on, but I think I didn't make myself clear enough with my last remark. I meant to change your code: ZnEntity>>contentType: object "We only allow assignment compatible with our designated mime type" | newType | newType := object asZnMimeType. (contentType notNil and: [ contentType matches: newType ]) ifTrue: [ ^ self ] ifFalse: [ (self class designatedMimeType isNil or: [ self class designatedMimeType matches: newType ]) ifTrue: [ contentType := newType ] ] to ZnEntity>>contentType: object "We only allow assignment compatible with our designated mime type" | newType | newType := object asZnMimeType. (contentType notNil and: [ contentType = newType ]) ifTrue: [ ^ self ] ifFalse: [ (self class designatedMimeType isNil or: [ self class designatedMimeType matches: newType ]) ifTrue: [ contentType := newType ] ]
and: ZnEntity class>>concreteSubclassForType: mimeType binary: forceBinary "Answer the concrete ZnEntity subclass that handles mimeType"
^ self allSubclasses detect: [ :each | each designatedMimeType matches: mimeType ] ifNone: [ (mimeType isBinary or: [ forceBinary ]) ifTrue: [ self byteArrayEntityClass ] ifFalse: [ self stringEntityClass ] ]
That one makes sense indeed.
With these changes, everything works like a charm again.
Kind regards,
Andy
On 04 Jun 2013, at 10:44, Sven Van Caekenberghe <sven@stfx.eu> wrote:
Hi Andy,
Yes, I forgot about ZnZincServerAdaptor when refactoring/changing ZnMimeType>>#= - sorry about that. The solution is actually quite easy: ZnMimeType>>#match: does a comparison while ignoring additional mime type parameters like charset, in addition to its wild card matching.
So the patch then would be:
ZnZincServerAdaptor>>requestFieldsFor: aZincRequest | fields | fields := WARequestFields new. (aZincRequest method = #POST and: [ aZincRequest hasEntity ]) ifTrue: [ (aZincRequest entity contentType match: ZnMimeType applicationFormUrlEncoded) ifTrue: [ fields addAll: aZincRequest entity fields ]. (aZincRequest entity contentType match: ZnMimeType multiPartFormData) ifTrue: [ fields addAll: (self convertMultipart: aZincRequest entity) ] ]. ^ fields
If you confirm that this works, I will check in the change later today.
Thanks again for the cristal clear bug report !
Regards,
Sven
On 04 Jun 2013, at 10:34, Andy Kellens <akellens@vub.ac.be> wrote:
Hi,
Yesterday, I stumbled across a bug when using the Zinc Seaside adaptor that resulted in that the post fields of a request were not present in the WARequest. I found out that the problem occurred when converting a Zinc request into a WARequest.
More particular: ZnZincServerAdaptor>>requestFieldsFor: aZincRequest | fields | fields := WARequestFields new. (aZincRequest method = #POST and: [ aZincRequest hasEntity ]) ifTrue: [ aZincRequest entity contentType = ZnMimeType applicationFormUrlEncoded ifTrue: [ fields addAll: aZincRequest entity fields ]. aZincRequest entity contentType = ZnMimeType multiPartFormData ifTrue: [ fields addAll: (self convertMultipart: aZincRequest entity) ] ]. ^ fields
This method checks if the Mime Type is either applicationFormUrlEncoded or multiPartFormData in order to extract the post fields.
Recently, ZnMimeType>>= got changed from: ZnMimeType>>= other ^ (self class == other class) and: [ self main = other main and: [ self sub = other sub ] ]
to: ZnMimeType>>= other ^ (self class == other class) and: [ self main = other main and: [ self sub = other sub and: [ self hasParameters not & other hasParameters not or: [ self parameters = other parameters ] ] ] ]
As my request had an additional parameter charset=utf-8, the equality with ZnMimeType applicationFormUrlEncoded failed and hence the fields were not extract.
While I assume that the comparison of the Mime type parameters in #= is there for a good reason, and the best way to fix this problem is to change the #requestFieldsFor: method in order to match the Mime type differently, I wanted to briefly check with the mailing list before opening an issue + submitting a patch.
Kind regards,
Andy
-- Sven Van Caekenberghe Proudly supporting Pharo http://pharo.org http://association.pharo.org http://consortium.pharo.org
On 04 Jun 2013, at 12:56, Sven Van Caekenberghe <sven@stfx.eu> wrote:
On 04 Jun 2013, at 11:20, Andy Kellens <akellens@vub.ac.be> wrote:
On 04 Jun 2013, at 11:13, Sven Van Caekenberghe <sven@stfx.eu> wrote:
On 04 Jun 2013, at 11:04, Andy Kellens <akellens@vub.ac.be> wrote:
Hi Sven,
Thanks for the quick response!
A slightly altered version of your patch (it is #matches: instead of #match:) results in that the code goes into the right branch.
ZnZincServerAdaptor>>requestFieldsFor: aZincRequest | fields | fields := WARequestFields new. (aZincRequest method = #POST and: [ aZincRequest hasEntity ]) ifTrue: [ (aZincRequest entity contentType matches: ZnMimeType applicationFormUrlEncoded) ifTrue: [ fields addAll: aZincRequest entity fields ]. (aZincRequest entity contentType matches: ZnMimeType multiPartFormData) ifTrue: [ fields addAll: (self convertMultipart: aZincRequest entity) ] ]. ^ fields
Yes, sorry for the typo.
Now, to fix my problem I still had to patch two other methods to also use #matches: instead of #= ZnEntity>>contentType: object "We only allow assignment compatible with our designated mime type"
| newType | newType := object asZnMimeType. (contentType notNil and:[contentType matches: newType]) ifTrue: [ ^ self ] ifFalse: [ (self class designatedMimeType isNil or: [ self class designatedMimeType matches: newType ]) ifTrue: [ contentType := newType ] ]
That makes sense, but I would say that the first condition should still use #= otherwise it would no longer be possible to change the mime type from text/plain to text/plain;charset=ascii for example (they types then match and nothing will be done). If you make that change, does it still work for you ?
If I change it back to #=, the entity of the request is no longer a ZnApplicationFormUrlEncodedEntity but a ZnByteArrayEntity. When retrieving the fields, I get a #DNU that ZnByteArrayEntity does not understand #fields.
I will go into a Seaside image myself later on, but I think I didn't make myself clear enough with my last remark. I meant to change your code:
ZnEntity>>contentType: object "We only allow assignment compatible with our designated mime type"
| newType | newType := object asZnMimeType. (contentType notNil and: [ contentType matches: newType ]) ifTrue: [ ^ self ] ifFalse: [ (self class designatedMimeType isNil or: [ self class designatedMimeType matches: newType ]) ifTrue: [ contentType := newType ] ]
to
ZnEntity>>contentType: object "We only allow assignment compatible with our designated mime type"
| newType | newType := object asZnMimeType. (contentType notNil and: [ contentType = newType ]) ifTrue: [ ^ self ] ifFalse: [ (self class designatedMimeType isNil or: [ self class designatedMimeType matches: newType ]) ifTrue: [ contentType := newType ] ]
Yes., that is exactly what I tried. When doing this, the entity of the request has the wrong type (it is a ZnByteArrayEntity instead of a ZnApplicationFormUrlEncodedEntity). I do not know the code well enough however to see whether using the #matches: is a good solution, or whether something has to be changed elsewhere.
and: ZnEntity class>>concreteSubclassForType: mimeType binary: forceBinary "Answer the concrete ZnEntity subclass that handles mimeType"
^ self allSubclasses detect: [ :each | each designatedMimeType matches: mimeType ] ifNone: [ (mimeType isBinary or: [ forceBinary ]) ifTrue: [ self byteArrayEntityClass ] ifFalse: [ self stringEntityClass ] ]
That one makes sense indeed.
With these changes, everything works like a charm again.
Kind regards,
Andy
On 04 Jun 2013, at 10:44, Sven Van Caekenberghe <sven@stfx.eu> wrote:
Hi Andy,
Yes, I forgot about ZnZincServerAdaptor when refactoring/changing ZnMimeType>>#= - sorry about that. The solution is actually quite easy: ZnMimeType>>#match: does a comparison while ignoring additional mime type parameters like charset, in addition to its wild card matching.
So the patch then would be:
ZnZincServerAdaptor>>requestFieldsFor: aZincRequest | fields | fields := WARequestFields new. (aZincRequest method = #POST and: [ aZincRequest hasEntity ]) ifTrue: [ (aZincRequest entity contentType match: ZnMimeType applicationFormUrlEncoded) ifTrue: [ fields addAll: aZincRequest entity fields ]. (aZincRequest entity contentType match: ZnMimeType multiPartFormData) ifTrue: [ fields addAll: (self convertMultipart: aZincRequest entity) ] ]. ^ fields
If you confirm that this works, I will check in the change later today.
Thanks again for the cristal clear bug report !
Regards,
Sven
On 04 Jun 2013, at 10:34, Andy Kellens <akellens@vub.ac.be> wrote:
Hi,
Yesterday, I stumbled across a bug when using the Zinc Seaside adaptor that resulted in that the post fields of a request were not present in the WARequest. I found out that the problem occurred when converting a Zinc request into a WARequest.
More particular: ZnZincServerAdaptor>>requestFieldsFor: aZincRequest | fields | fields := WARequestFields new. (aZincRequest method = #POST and: [ aZincRequest hasEntity ]) ifTrue: [ aZincRequest entity contentType = ZnMimeType applicationFormUrlEncoded ifTrue: [ fields addAll: aZincRequest entity fields ]. aZincRequest entity contentType = ZnMimeType multiPartFormData ifTrue: [ fields addAll: (self convertMultipart: aZincRequest entity) ] ]. ^ fields
This method checks if the Mime Type is either applicationFormUrlEncoded or multiPartFormData in order to extract the post fields.
Recently, ZnMimeType>>= got changed from: ZnMimeType>>= other ^ (self class == other class) and: [ self main = other main and: [ self sub = other sub ] ]
to: ZnMimeType>>= other ^ (self class == other class) and: [ self main = other main and: [ self sub = other sub and: [ self hasParameters not & other hasParameters not or: [ self parameters = other parameters ] ] ] ]
As my request had an additional parameter charset=utf-8, the equality with ZnMimeType applicationFormUrlEncoded failed and hence the fields were not extract.
While I assume that the comparison of the Mime type parameters in #= is there for a good reason, and the best way to fix this problem is to change the #requestFieldsFor: method in order to match the Mime type differently, I wanted to briefly check with the mailing list before opening an issue + submitting a patch.
Kind regards,
Andy
-- Sven Van Caekenberghe Proudly supporting Pharo http://pharo.org http://association.pharo.org http://consortium.pharo.org
Development by proxy is hard ;-) OK: I have a Pharo 2.0 + Seaside 3.1 image with Zn all up to date. What is the simplest thing that I can do to trigger the problem ? Can I use one of the builtin functional tests ? On 04 Jun 2013, at 13:40, Andy Kellens <akellens@vub.ac.be> wrote:
On 04 Jun 2013, at 12:56, Sven Van Caekenberghe <sven@stfx.eu> wrote:
On 04 Jun 2013, at 11:20, Andy Kellens <akellens@vub.ac.be> wrote:
On 04 Jun 2013, at 11:13, Sven Van Caekenberghe <sven@stfx.eu> wrote:
On 04 Jun 2013, at 11:04, Andy Kellens <akellens@vub.ac.be> wrote:
Hi Sven,
Thanks for the quick response!
A slightly altered version of your patch (it is #matches: instead of #match:) results in that the code goes into the right branch.
ZnZincServerAdaptor>>requestFieldsFor: aZincRequest | fields | fields := WARequestFields new. (aZincRequest method = #POST and: [ aZincRequest hasEntity ]) ifTrue: [ (aZincRequest entity contentType matches: ZnMimeType applicationFormUrlEncoded) ifTrue: [ fields addAll: aZincRequest entity fields ]. (aZincRequest entity contentType matches: ZnMimeType multiPartFormData) ifTrue: [ fields addAll: (self convertMultipart: aZincRequest entity) ] ]. ^ fields
Yes, sorry for the typo.
Now, to fix my problem I still had to patch two other methods to also use #matches: instead of #= ZnEntity>>contentType: object "We only allow assignment compatible with our designated mime type"
| newType | newType := object asZnMimeType. (contentType notNil and:[contentType matches: newType]) ifTrue: [ ^ self ] ifFalse: [ (self class designatedMimeType isNil or: [ self class designatedMimeType matches: newType ]) ifTrue: [ contentType := newType ] ]
That makes sense, but I would say that the first condition should still use #= otherwise it would no longer be possible to change the mime type from text/plain to text/plain;charset=ascii for example (they types then match and nothing will be done). If you make that change, does it still work for you ?
If I change it back to #=, the entity of the request is no longer a ZnApplicationFormUrlEncodedEntity but a ZnByteArrayEntity. When retrieving the fields, I get a #DNU that ZnByteArrayEntity does not understand #fields.
I will go into a Seaside image myself later on, but I think I didn't make myself clear enough with my last remark. I meant to change your code:
ZnEntity>>contentType: object "We only allow assignment compatible with our designated mime type"
| newType | newType := object asZnMimeType. (contentType notNil and: [ contentType matches: newType ]) ifTrue: [ ^ self ] ifFalse: [ (self class designatedMimeType isNil or: [ self class designatedMimeType matches: newType ]) ifTrue: [ contentType := newType ] ]
to
ZnEntity>>contentType: object "We only allow assignment compatible with our designated mime type"
| newType | newType := object asZnMimeType. (contentType notNil and: [ contentType = newType ]) ifTrue: [ ^ self ] ifFalse: [ (self class designatedMimeType isNil or: [ self class designatedMimeType matches: newType ]) ifTrue: [ contentType := newType ] ]
Yes., that is exactly what I tried. When doing this, the entity of the request has the wrong type (it is a ZnByteArrayEntity instead of a ZnApplicationFormUrlEncodedEntity). I do not know the code well enough however to see whether using the #matches: is a good solution, or whether something has to be changed elsewhere.
and: ZnEntity class>>concreteSubclassForType: mimeType binary: forceBinary "Answer the concrete ZnEntity subclass that handles mimeType"
^ self allSubclasses detect: [ :each | each designatedMimeType matches: mimeType ] ifNone: [ (mimeType isBinary or: [ forceBinary ]) ifTrue: [ self byteArrayEntityClass ] ifFalse: [ self stringEntityClass ] ]
That one makes sense indeed.
With these changes, everything works like a charm again.
Kind regards,
Andy
On 04 Jun 2013, at 10:44, Sven Van Caekenberghe <sven@stfx.eu> wrote:
Hi Andy,
Yes, I forgot about ZnZincServerAdaptor when refactoring/changing ZnMimeType>>#= - sorry about that. The solution is actually quite easy: ZnMimeType>>#match: does a comparison while ignoring additional mime type parameters like charset, in addition to its wild card matching.
So the patch then would be:
ZnZincServerAdaptor>>requestFieldsFor: aZincRequest | fields | fields := WARequestFields new. (aZincRequest method = #POST and: [ aZincRequest hasEntity ]) ifTrue: [ (aZincRequest entity contentType match: ZnMimeType applicationFormUrlEncoded) ifTrue: [ fields addAll: aZincRequest entity fields ]. (aZincRequest entity contentType match: ZnMimeType multiPartFormData) ifTrue: [ fields addAll: (self convertMultipart: aZincRequest entity) ] ]. ^ fields
If you confirm that this works, I will check in the change later today.
Thanks again for the cristal clear bug report !
Regards,
Sven
On 04 Jun 2013, at 10:34, Andy Kellens <akellens@vub.ac.be> wrote:
Hi,
Yesterday, I stumbled across a bug when using the Zinc Seaside adaptor that resulted in that the post fields of a request were not present in the WARequest. I found out that the problem occurred when converting a Zinc request into a WARequest.
More particular: ZnZincServerAdaptor>>requestFieldsFor: aZincRequest | fields | fields := WARequestFields new. (aZincRequest method = #POST and: [ aZincRequest hasEntity ]) ifTrue: [ aZincRequest entity contentType = ZnMimeType applicationFormUrlEncoded ifTrue: [ fields addAll: aZincRequest entity fields ]. aZincRequest entity contentType = ZnMimeType multiPartFormData ifTrue: [ fields addAll: (self convertMultipart: aZincRequest entity) ] ]. ^ fields
This method checks if the Mime Type is either applicationFormUrlEncoded or multiPartFormData in order to extract the post fields.
Recently, ZnMimeType>>= got changed from: ZnMimeType>>= other ^ (self class == other class) and: [ self main = other main and: [ self sub = other sub ] ]
to: ZnMimeType>>= other ^ (self class == other class) and: [ self main = other main and: [ self sub = other sub and: [ self hasParameters not & other hasParameters not or: [ self parameters = other parameters ] ] ] ]
As my request had an additional parameter charset=utf-8, the equality with ZnMimeType applicationFormUrlEncoded failed and hence the fields were not extract.
While I assume that the comparison of the Mime type parameters in #= is there for a good reason, and the best way to fix this problem is to change the #requestFieldsFor: method in order to match the Mime type differently, I wanted to briefly check with the mailing list before opening an issue + submitting a patch.
Kind regards,
Andy
-- Sven Van Caekenberghe Proudly supporting Pharo http://pharo.org http://association.pharo.org http://consortium.pharo.org
On 04 Jun 2013, at 14:13, Sven Van Caekenberghe <sven@stfx.eu> wrote:
OK: I have a Pharo 2.0 + Seaside 3.1 image with Zn all up to date. What is the simplest thing that I can do to trigger the problem ? Can I use one of the builtin functional tests ?
Found it, I broke about every Seaside request ;-)
Andy, On 04 Jun 2013, at 14:42, Sven Van Caekenberghe <sven@stfx.eu> wrote:
On 04 Jun 2013, at 14:13, Sven Van Caekenberghe <sven@stfx.eu> wrote:
OK: I have a Pharo 2.0 + Seaside 3.1 image with Zn all up to date. What is the simplest thing that I can do to trigger the problem ? Can I use one of the builtin functional tests ?
Found it, I broke about every Seaside request ;-)
Well, it took longer than expected because my Seaside image contained leftovers from experiments that I long forgot about. Anyway, the following two version should be good: === Name: Zinc-HTTP-SvenVanCaekenberghe.367 Author: SvenVanCaekenberghe Time: 4 June 2013, 4:28:29 am UUID: 04dcc6f1-361f-46c1-a734-746b71460eb2 Ancestors: Zinc-HTTP-SvenVanCaekenberghe.366 Introduction of ZnEntity class>>#matches: to fix ZnEntity class>>#concreteSubclassForType:binary: and ZnEntity>>#contentType: (Thanks Andy Kellens) === Name: Zinc-Seaside-SvenVanCaekenberghe.40 Author: SvenVanCaekenberghe Time: 4 June 2013, 4:24:57 am UUID: a6c5f1db-16b3-46ef-912f-95f54235c518 Ancestors: Zinc-Seaside-SeanDeNigris.39 Modified ZnZincServerAdaptor>>#requestBodyFor: and #requestFieldsFor: to use ZnMimeType>>#matches: after a change in the semantics of ZnMimeType>>#= (mime type parameters like charset are now relevant) (Thx Andy Kellens) === Please confirm that it works for you. Regards, Sven -- Sven Van Caekenberghe Proudly supporting Pharo http://pharo.org http://association.pharo.org http://consortium.pharo.org
Hi Sven, I just tested your new version of Zinc-HTTP and everything works! Thanks, Andy On 04 Jun 2013, at 16:44, Sven Van Caekenberghe <sven@stfx.eu> wrote:
Andy,
On 04 Jun 2013, at 14:42, Sven Van Caekenberghe <sven@stfx.eu> wrote:
On 04 Jun 2013, at 14:13, Sven Van Caekenberghe <sven@stfx.eu> wrote:
OK: I have a Pharo 2.0 + Seaside 3.1 image with Zn all up to date. What is the simplest thing that I can do to trigger the problem ? Can I use one of the builtin functional tests ?
Found it, I broke about every Seaside request ;-)
Well, it took longer than expected because my Seaside image contained leftovers from experiments that I long forgot about. Anyway, the following two version should be good:
=== Name: Zinc-HTTP-SvenVanCaekenberghe.367 Author: SvenVanCaekenberghe Time: 4 June 2013, 4:28:29 am UUID: 04dcc6f1-361f-46c1-a734-746b71460eb2 Ancestors: Zinc-HTTP-SvenVanCaekenberghe.366
Introduction of ZnEntity class>>#matches: to fix ZnEntity class>>#concreteSubclassForType:binary: and ZnEntity>>#contentType: (Thanks Andy Kellens) === Name: Zinc-Seaside-SvenVanCaekenberghe.40 Author: SvenVanCaekenberghe Time: 4 June 2013, 4:24:57 am UUID: a6c5f1db-16b3-46ef-912f-95f54235c518 Ancestors: Zinc-Seaside-SeanDeNigris.39
Modified ZnZincServerAdaptor>>#requestBodyFor: and #requestFieldsFor: to use ZnMimeType>>#matches: after a change in the semantics of ZnMimeType>>#= (mime type parameters like charset are now relevant) (Thx Andy Kellens) ===
Please confirm that it works for you.
Regards,
Sven
-- Sven Van Caekenberghe Proudly supporting Pharo http://pharo.org http://association.pharo.org http://consortium.pharo.org
Great, thank you for the help ! On 04 Jun 2013, at 19:05, Andy Kellens <akellens@vub.ac.be> wrote:
Hi Sven,
I just tested your new version of Zinc-HTTP and everything works!
Thanks,
Andy
On 04 Jun 2013, at 16:44, Sven Van Caekenberghe <sven@stfx.eu> wrote:
Andy,
On 04 Jun 2013, at 14:42, Sven Van Caekenberghe <sven@stfx.eu> wrote:
On 04 Jun 2013, at 14:13, Sven Van Caekenberghe <sven@stfx.eu> wrote:
OK: I have a Pharo 2.0 + Seaside 3.1 image with Zn all up to date. What is the simplest thing that I can do to trigger the problem ? Can I use one of the builtin functional tests ?
Found it, I broke about every Seaside request ;-)
Well, it took longer than expected because my Seaside image contained leftovers from experiments that I long forgot about. Anyway, the following two version should be good:
=== Name: Zinc-HTTP-SvenVanCaekenberghe.367 Author: SvenVanCaekenberghe Time: 4 June 2013, 4:28:29 am UUID: 04dcc6f1-361f-46c1-a734-746b71460eb2 Ancestors: Zinc-HTTP-SvenVanCaekenberghe.366
Introduction of ZnEntity class>>#matches: to fix ZnEntity class>>#concreteSubclassForType:binary: and ZnEntity>>#contentType: (Thanks Andy Kellens) === Name: Zinc-Seaside-SvenVanCaekenberghe.40 Author: SvenVanCaekenberghe Time: 4 June 2013, 4:24:57 am UUID: a6c5f1db-16b3-46ef-912f-95f54235c518 Ancestors: Zinc-Seaside-SeanDeNigris.39
Modified ZnZincServerAdaptor>>#requestBodyFor: and #requestFieldsFor: to use ZnMimeType>>#matches: after a change in the semantics of ZnMimeType>>#= (mime type parameters like charset are now relevant) (Thx Andy Kellens) ===
Please confirm that it works for you.
Regards,
Sven
-- Sven Van Caekenberghe Proudly supporting Pharo http://pharo.org http://association.pharo.org http://consortium.pharo.org
participants (2)
-
Andy Kellens -
Sven Van Caekenberghe