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