On 04 Jun 2013, at 11:04, Andy Kellens <[email protected]> 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 <[email protected]> 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 <[email protected]> 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
>> 
>> 
>> 
>> 
>> 
> 
> 


Reply via email to