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 <[email protected]> wrote: > > On 04 Jun 2013, at 12:56, Sven Van Caekenberghe <[email protected]> wrote: > >> >> On 04 Jun 2013, at 11:20, Andy Kellens <[email protected]> wrote: >> >>> >>> On 04 Jun 2013, at 11:13, Sven Van Caekenberghe <[email protected]> wrote: >>> >>>> >>>> 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 ? >>> >>> 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 <[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 >>>>>> >>>>>> >>>>>> >>>>>> >>>>>> >>>>> >>>>> >>>> >>>> >>> >>> >> >> > >
