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 >>>>> >>>>> >>>>> >>>>> >>>>> >>>> >>>> >>> >>> >> >> > >
